cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Grieve <agri...@chromium.org>
Subject Re: [JS] merging to master for cordova-js and cordova-plugman
Date Fri, 25 Apr 2014 18:05:35 GMT
On Fri, Apr 25, 2014 at 1:35 PM, Michal Mocny <mmocny@chromium.org> wrote:
> On Fri, Apr 25, 2014 at 1:27 PM, Anis KADRI <anis.kadri@gmail.com> wrote:
>
>> Contacts is known to not work because some modules
>> require('./ContactError') instead of
>> ('org.apache.cordova.contacts.ContactError') like most other modules do. I
>> didn't bother to fix yet it because it was an exception not a rule.
>> I didn't try any of the chrome plugins. They might be doing the same sort
>> of thing.

Just wanted to be clear here that "fix" means allow the "./syntax",
not convert to abs syntax. File plugin uses this extensively, and
we've supported it for quite a while.


>> I didn't noticed any major performance issues when comparing old plugman
>> and new plugman with browserify. I am not using CLI though.

As a litmus test, I'd suggest running the createmobilespec.js script
within mobile-spec.

>> Thanks for giving me the time to look into this before reverting my
>> changes. Much appreciated. Besides reverting cordova-js was completely
>> unnecessary because I only added a task to compile a browserify bundle
>> (everything else was not touched).

I think it's important to keep the continuous build green, and
cordova-js was broken:
https://issues.apache.org/jira/browse/CB-6515

To re-do your changes, just git revert the revert commit and carry on.
It's important that everyone else isn't blocked by master being
broken.


>>
>
> All the fixes will probably take more than an hour, and shouldn't block
> master.  Why the rush to merge in?
>
>
>>
>>
>>
>> On Fri, Apr 25, 2014 at 8:13 AM, Andrew Grieve <agrieve@chromium.org>wrote:
>>
>>> reverted the merge commits to plugman & js.
>>>
>>> On Fri, Apr 25, 2014 at 9:43 AM, Michal Mocny <mmocny@chromium.org>
>>> wrote:
>>> > Also Anis, would it please be possible to squash most of these commits
>>> into
>>> > a few units of standalone work?  Probably best way to do that is create
>>> a
>>> > new local branch from your old branch point, apply the diffs from
>>> > browserify branch, rebase -i them, then push that branch (this way you
>>> > don't need to push with force).
>>> >
>>> > -Michal
>>> >
>>> >
>>> > On Fri, Apr 25, 2014 at 9:30 AM, Michal Mocny <mmocny@chromium.org>
>>> wrote:
>>> >
>>> >>
>>> >>
>>> >>
>>> >> On Fri, Apr 25, 2014 at 12:53 AM, Michal Mocny <mmocny@chromium.org
>>> >wrote:
>>> >>
>>> >>> TLDR; Ran a bunch of experiments tonight.  I think this is too early
>>> to
>>> >>> merge into master.  We pointed out several issues in previous threads
>>> and
>>> >>> seems they were ignored.
>>> >>>
>>> >>> Few quick comments from trying this:
>>> >>> - cordova-js is a new dependency of plugman, and needs to be npm
>>> linked
>>> >>> to local dev version
>>> >>> - Seems we run browserify after each plugin add (possibly due to
an
>>> >>> auto-prepare?) so creating projects like mobilespec or any mobile
>>> chrome
>>> >>> app is now *much* slower (measured in minutes)
>>> >>>   - Each cordova prepare now takes 6.5s on a very small project
:'(
>>> >>>
>>> >> (reverted changes locally, old prepare takes <0.5s on same project,
>>> which
>>> >> is still slow!)
>>> >>
>>> >>
>>> >>> Things that are currently found broken:
>>> >>> - prepare step fails for my cordova testing application after
>>> >>> installing org.apache.cordova.contacts, and
>>> >>> - prepare step fails for *all* cca apps because of same error as
>>> above,
>>> >>> but for chrome.runtime plugin :(
>>> >>>   - These issues seem due to js-modules not being browserify-ed
>>> properly.
>>> >>>  It may be that both are bad modules (?), but it used to work fine!
>>> >>>
>>> >>> I did get a few apps running fine, so at least we got that going
for
>>> us ;)
>>> >>>
>>> >>> Still to do:
>>> >>> - track impact tp startup time
>>> >>> - see if there aren't any plugins with subtle bugs due to auto-runs
>>> >>> behaviour
>>> >>>
>>> >>> -Michal
>>> >>>
>>> >>>
>>> >>> On Thu, Apr 24, 2014 at 9:34 PM, Andrew Grieve <agrieve@chromium.org
>>> >wrote:
>>> >>>
>>> >>>> Cool! Does no impact mean that browserify is still not used
by
>>> >>>> default, or does it mean that it's backward compatible?
>>> >>>>
>>> >>>> Failing specs sounds like impact...
>>> >>>>
>>> >>>> And it does look like medic is failing due to browserify-type
things:
>>> >>>> http://108.170.217.131:8010/waterfall
>>> >>>>
>>> >>>> Unless you feel like powering through this tonight, I'll probably
>>> >>>> revert in the morning so that our continuous build can stay
green.
>>> >>>>
>>> >>>> On Thu, Apr 24, 2014 at 6:06 PM, Brian LeRoux <b@brian.io>
wrote:
>>> >>>> > \o/
>>> >>>> >
>>> >>>> >
>>> >>>> > On Thu, Apr 24, 2014 at 2:30 PM, Anis KADRI <anis@apache.org>
>>> wrote:
>>> >>>> >
>>> >>>> >> I just merged both browserify branches into master.
There should
>>> be no
>>> >>>> >> impact.
>>> >>>> >> Right now most specs pass expect for File, FileTransfer,
Media and
>>> >>>> Contacts
>>> >>>> >> due to some issues with merges/clobbers and I am looking
into
>>> those.
>>> >>>> >>
>>> >>>> >> Also, I got rid of the project cache condition in plugman
that was
>>> >>>> >> preventing iOS frameworks from being added (CB-6441)
>>> >>>> >>
>>> >>>> >> Anis
>>> >>>> >>
>>> >>>>
>>> >>>
>>> >>>
>>> >>
>>>
>>
>>

Mime
View raw message