I also think Fil should be more chill ;) Just kidding, obviously the conclusions are satisfactory. Clearly, writing is hard.
-Michal On Thu, Feb 21, 2013 at 2:24 PM, Andrew Grieve <agri...@chromium.org> wrote: > I think so! Thanks everyone for working through this with me. > > > On Thu, Feb 21, 2013 at 2:08 PM, Filip Maj <f...@adobe.com> wrote: > > > Hope the conclusions in this thread are satisfactory. > > > > Andrew, I apologize for the tense tone in my emails yesterday. I should > > have been more chill about it. > > > > On 2/20/13 7:52 PM, "Michael Brooks" <mich...@michaelbrooks.ca> wrote: > > > > >Sounds awesome Andrew, thanks for the concise recap. > > > > > >I agree, JIRA isn't the ideal solution when considering today's options > > >around code conversation, but it's what we have as an Apache project. > > >C'est > > >le vie. > > > > > >Regardless, if you need a platform to do a task, then JIRA is the > answer. > > >If you need to discuss something before deciding on the tasks to do, > then > > >the mailing-list is the answer. > > > > > >Michael > > > > > >On Wed, Feb 20, 2013 at 1:58 PM, Andrew Grieve <agri...@chromium.org> > > >wrote: > > > > > >> Recap: > > >> I tested the common changes against iOS & Android and checked them > in. I > > >> also checked in the blackberry ones, since they contain better test > > >> coverage in cordova-js than other platforms. > > >> > > >> I then emailed out asking if anyone could test the remaining changes > on > > >>the > > >> branch against WP and webos. After 8 days of silence, I merged them in > > >> hopes that sometime between now and the 2.6 release someone will run > the > > >> tests and discover if the changes broke anything. > > >> > > >> > > >> Outcome: > > >> -Instead of the mailing-list, we'll use JIRA issues to assign specific > > >> owners to the job of testing out these changes when they come up. > > >> > > >> > > >> What to do now: > > >> I'll create two sub-tasks of CB-2227. One for WP, one for webos. > > >> > > >> > > >> Sound right / good? > > >> > > >> > > >> > > >> > > >> > > >> On Wed, Feb 20, 2013 at 4:32 PM, Michal Mocny <mmo...@chromium.org> > > >>wrote: > > >> > > >> > Agree with where this conversation is going. I do think a "call to > > >> action" > > >> > for review is important (esp given the number of platforms) and > > >>perhaps > > >> > JIRA isn't the absolute worst way to do it ;) > > >> > > > >> > Another question: Are there plans to expand CI to other platforms? > > >> Support > > >> > requesting tests for a remote feature branch? This may remove some > > >> strain > > >> > on testers for the less popular platforms. > > >> > > > >> > > > >> > On Wed, Feb 20, 2013 at 4:29 PM, Filip Maj <f...@adobe.com> wrote: > > >> > > > >> > > Good call Mike. Moving this sort of stuff to JIRA (and bringing > > >>back to > > >> > > list when necessary) makes a lot of sense. > > >> > > > > >> > > On 2/20/13 1:27 PM, "Michael Brooks" <mich...@michaelbrooks.ca> > > >>wrote: > > >> > > > > >> > > >> > > >> > > >> If the "can you guys test my changes" answer is "no", then it'd > > >>be > > >> > > >>great to > > >> > > >> hear a "no" instead of 8 days of silence. That said, I think > > >>we'd be > > >> > > >>able > > >> > > >> to move faster if we just took some time to review/test each > > >>others' > > >> > > >> changes when necessary. We do this when processing pull > requests > > >> from > > >> > > >> external devs, so why not do this for each other? > > >> > > > > > >> > > > > > >> > > >Totally valid and I support the suggestion of code reviews (as > > >>long as > > >> > its > > >> > > >not a hinderance to commit ease). > > >> > > > > > >> > > >It would be nice if we can bring conversations closer to the code > > >>(via > > >> > > >GitHub pull requests and code comments) instead of blasting > emails > > >>at > > >> > each > > >> > > >other. I assume using GitHub more is against the Apache Way? We > > >>could > > >> > > >bring > > >> > > >conversations more into to JIRA. For this particular example, > > >>Andrew > > >> > could > > >> > > >create a JIRA issue and assign subtasks for the major platforms > to > > >> test > > >> > to > > >> > > >branch and report back any issues. > > >> > > > > > >> > > >Michael > > >> > > > > > >> > > >On Wed, Feb 20, 2013 at 1:07 PM, Andrew Grieve > > >><agri...@chromium.org> > > >> > > >wrote: > > >> > > > > > >> > > >> I agree with your sentiments, but I think it's impractical in > > >> > practice. > > >> > > >>We > > >> > > >> have ~11 platforms, and any change to common js affects them > all. > > >> > > >> > > >> > > >> In this case, I would need to learn how to build & run on > webos, > > >> > tizen, > > >> > > >>wp7 > > >> > > >> and windowphone, as well as buying the required hardware to do > > >>so. A > > >> > > >>tall > > >> > > >> order for some refactoring changes. > > >> > > >> > > >> > > >> If the "can you guys test my changes" answer is "no", then it'd > > >>be > > >> > > >>great to > > >> > > >> hear a "no" instead of 8 days of silence. That said, I think > > >>we'd be > > >> > > >>able > > >> > > >> to move faster if we just took some time to review/test each > > >>others' > > >> > > >> changes when necessary. We do this when processing pull > requests > > >> from > > >> > > >> external devs, so why not do this for each other? > > >> > > >> > > >> > > >> > > >> > > >> > > >> > > >> > > >> > > >> > > >> > > >> > > >> > > >> On Wed, Feb 20, 2013 at 3:47 PM, Michael Brooks > > >> > > >><mich...@michaelbrooks.ca > > >> > > >> >wrote: > > >> > > >> > > >> > > >> > Agreed. Please take responsibility and test your code on > > >>devices > > >> > > >>(ideally > > >> > > >> > not simulators). > > >> > > >> > > > >> > > >> > If your change impacts multiple platforms, have it tested on > > >>those > > >> > > >>before > > >> > > >> > pushing to master. > > >> > > >> > > > >> > > >> > On Wed, Feb 20, 2013 at 12:42 PM, Filip Maj <f...@adobe.com> > > >> wrote: > > >> > > >> > > > >> > > >> > > Andrew did you test it on a device? Don't think "hey guys > can > > >> you > > >> > > >>test > > >> > > >> my > > >> > > >> > > changes" is a sustainable approach > > >> > > >> > > > > >> > > >> > > On 2/20/13 12:19 PM, "Andrew Grieve" <agri...@chromium.org > > > > >> > wrote: > > >> > > >> > > > > >> > > >> > > >Okay, I've interpreted the silence as a "just go ahead and > > >> merge > > >> > it > > >> > > >> and > > >> > > >> > > >people will complain if it's broken". > > >> > > >> > > > > > >> > > >> > > >Seeing as we've now cut the next branch, I figured it was > a > > >> good > > >> > > >>time > > >> > > >> to > > >> > > >> > > >merge these remaining changes in. So, I did. Please let me > > >>know > > >> > if > > >> > > >> > symbols > > >> > > >> > > >aren't working. > > >> > > >> > > > > > >> > > >> > > > > > >> > > >> > > >On Tue, Feb 12, 2013 at 2:39 PM, Andrew Grieve > > >> > > >><agri...@chromium.org> > > >> > > >> > > >wrote: > > >> > > >> > > > > > >> > > >> > > >> I've just merged in most of the changes for this > > >>(CB-2227). > > >> > > >>Again, > > >> > > >> the > > >> > > >> > > >> goal here was to move all plugin-specific logic out of > > >>common > > >> > > >>files. > > >> > > >> > > >>It's > > >> > > >> > > >> not the end-game solution, but a step in the right > > >>direction. > > >> > > >> > > >> > > >> > > >> > > >> There are still some changes left on the symbolmapping > > >>branch > > >> > > >>that > > >> > > >> > > >>affect > > >> > > >> > > >> windows & webos. If there's someone who knows how, it > > >>would > > >> be > > >> > > >>great > > >> > > >> > if > > >> > > >> > > >>you > > >> > > >> > > >> could try merging in the branch and ensure mobile-spec > is > > >> still > > >> > > >> > working > > >> > > >> > > >> with the changes. If so, these changes can also be > merged > > >> into > > >> > > >> master. > > >> > > >> > > >> > > >> > > >> > > >> > > >> > > >> > > >> > > >> > > >> > > >> On Tue, Jan 29, 2013 at 3:38 PM, Andrew Grieve > > >> > > >> > > >><agri...@chromium.org>wrote: > > >> > > >> > > >> > > >> > > >> > > >>> This is now finished in the branch. There is now *no* > > >>plugin > > >> > > >>logic > > >> > > >> > left > > >> > > >> > > >>> in common.js, nor in any platform.js files. > > >> > > >> > > >>> > > >> > > >> > > >>> > > >> > > >> > > >>> > > >> > > >> > > >>> > > >> > > >> > > > > >> > > >> > > >> > > > > >> > > https://git-wip-us.apache.org/repos/asf?p=cordova-js.git;a=shortlog;h=re > > >> > > >> > > >>>fs/heads/symbolmapping > > >> > > >> > > >>> > > >> > > >> > > >>> There is one exception, and that's things like the > "app" > > >> > plugin, > > >> > > >> > where > > >> > > >> > > >>> it's not really a plugin that a platform can live > > >>without. > > >> > > >> > > >>> > > >> > > >> > > >>> Changes of interest: > > >> > > >> > > >>> 1. In tests, I've added helpers for stubbing out > modules > > >>& > > >> for > > >> > > >> > stubbing > > >> > > >> > > >>> out properties. I used this to be able to undo symbol > > >> mapping > > >> > > >> within > > >> > > >> > > >>>tests. > > >> > > >> > > >>> > > >> > > >> > > >>> var propertyreplacer = > > >>require('cordova/propertyreplacer'); > > >> > > >> > > >>> propertyreplacer.stub(platform, 'id', 'test'); > > >> > > >> > > >>> > > >> > > >> > > >>> var modulereplacer = require('cordova/modulereplacer'); > > >> > > >> > > >>> modulereplacer.replace('cordova/platform', {id:'test', > > >> > > >> > > >>> initialize:createSpy()}); > > >> > > >> > > >>> > > >> > > >> > > >>> 2. Loading plugins by name (aka, looping through all > > >>defined > > >> > > >> modules > > >> > > >> > > >>>and > > >> > > >> > > >>> loading the ones that have names that match a pattern). > > >> > > >> > > >>> A) "symbols" Modules that have the name "symbols" are > > >>loaded > > >> > to > > >> > > >> > define > > >> > > >> > > >>> their plugin's module->JS symbol mappings > > >> > > >> (merges/clobbers/defaults). > > >> > > >> > > >>>On > > >> > > >> > > >>> Blackberry, sub-platform symbol files are called > > >> "bbsymbols". > > >> > > >> > > >>> B) "plugininit" Modules that have the name "plugininit" > > >>are > > >> > > >>loaded > > >> > > >> to > > >> > > >> > > >>> perform any custom start-up logic. > > >> > > >> > > >>> C) "*Proxy" On Windows8, modules that end with "Proxy" > > >>are > > >> > > >>loaded > > >> > > >> on > > >> > > >> > > >>> start-up. > > >> > > >> > > >>> > > >> > > >> > > >>> I don't love the looping-through-module-names approach, > > >>but > > >> > > >>thought > > >> > > >> > it > > >> > > >> > > >>> was a good initial solution while we talk about better > > >> ideas. > > >> > > >>To do > > >> > > >> > > >>>this, I > > >> > > >> > > >>> had to make the moduleMap exported, which it wasn't > > >>before. > > >> > > >> Certainly > > >> > > >> > > >>> interested to hear if this is a really bad idea, and > what > > >> > > >> > alternatives > > >> > > >> > > >>>we > > >> > > >> > > >>> could use going forward. > > >> > > >> > > >>> > > >> > > >> > > >>> > > >> > > >> > > >>> > > >> > > >> > > >>> > > >> > > >> > > >>> > > >> > > >> > > >>> > > >> > > >> > > >>> On Thu, Jan 17, 2013 at 12:45 PM, Andrew Grieve > > >> > > >> > > >>><agri...@chromium.org>wrote: > > >> > > >> > > >>> > > >> > > >> > > >>>> Pushed up the change with the File plugin being > > >>registered > > >> in > > >> > > >>this > > >> > > >> > new > > >> > > >> > > >>>> way. Please let me know if you have concerns about it, > > >> since > > >> > > >>the > > >> > > >> > next > > >> > > >> > > >>>>step > > >> > > >> > > >>>> is moving over other plugin APIs, which is boring work > > >>:P. > > >> > > >> > > >>>> > > >> > > >> > > >>>> Also, let's move any discussion into the JIRA issue: > > >> > > >> > > >>>> https://issues.apache.org/jira/browse/CB-2227 > > >> > > >> > > >>>> > > >> > > >> > > >>>> > > >> > > >> > > >>>> > > >> > > >> > > >>>> On Wed, Jan 16, 2013 at 4:35 PM, Andrew Grieve > > >> > > >> > > >>>><agri...@chromium.org>wrote: > > >> > > >> > > >>>> > > >> > > >> > > >>>>> Branch started! > > >> > > >> > > >>>>> > > >> > > >> > > >>>>> I've completed steps 1 & 2. > > >> > > >> > > >>>>> > > >> > > >> > > >>>>> > > >> > > >> > > >>>>> > > >> > > >> > > >>>>> > > >> > > >> > > > > >> > > >> > > >> > https://git-wip-us.apache.org/repos/asf?p=cordova-js.git;a=shortlog;h= > > >> > > >> > > >>>>>refs/heads/symbolmapping > > >> > > >> > > >>>>> > > >> > > >> > > >>>>> > > >> > > >> > > >>>>> On Wed, Jan 16, 2013 at 1:39 PM, Filip Maj > > >><f...@adobe.com > > >> > > > >> > > >> wrote: > > >> > > >> > > >>>>> > > >> > > >> > > >>>>>> This all seems reasonable. Shall we start a branch? > > >> > > >> > > >>>>>> > > >> > > >> > > >>>>>> On 1/15/13 2:47 PM, "Andrew Grieve" > > >><agri...@google.com> > > >> > > >>wrote: > > >> > > >> > > >>>>>> > > >> > > >> > > >>>>>> >Sorry to dump another large email on the list, but > > >>I'm > > >> > > >>hoping > > >> > > >> > this > > >> > > >> > > >>>>>> one is > > >> > > >> > > >>>>>> >at least less controversial :). I wrote up a plan > for > > >> > moving > > >> > > >> > > >>>>>> >module->symbol > > >> > > >> > > >>>>>> >mapping out of common.js & platform.js and into > > >> individual > > >> > > >> > plugins. > > >> > > >> > > >>>>>> > > > >> > > >> > > >>>>>> >If you have feedback/comments, let me know. > > >> > > >> > > >>>>>> > > > >> > > >> > > >>>>>> >* Goals: > > >> > > >> > > >>>>>> > > > >> > > >> > > >>>>>> > - Change from listing module->symbol mapping > > >>within > > >> > > >> common.js > > >> > > >> > & > > >> > > >> > > >>>>>> > platform.js, to listing this within the plugins > > >> > > >>themselves. > > >> > > >> > > >>>>>> > - Support apps that don't want us to clobber > > >>global > > >> > > >>symbols. > > >> > > >> > > >>>>>> > - aka, allow module->symbol mapping to be > > >>turned > > >> off > > >> > > >> > > >>>>>> > - Allow retrieval of clobbered globals > > >> > > >> > > >>>>>> > - Currently modules save it themselves when > > >>they > > >> are > > >> > > >> loaded > > >> > > >> > > >>>>>> > - This won't work (reliably) for saving > > >>references > > >> > to > > >> > > >> > globals > > >> > > >> > > >>>>>> > overridden by other modules > > >> > > >> > > >>>>>> > - This gets in the way of the idea of > > >>lazy-loading > > >> > > >> modules > > >> > > >> > > >>>>>>via > > >> > > >> > > >>>>>> >getters > > >> > > >> > > >>>>>> > - Support the use of other module loaders > > >> > > >> > > >>>>>> > - So... don't do crazy things at require() > > >>time. > > >> > > >> > > >>>>>> > > > >> > > >> > > >>>>>> > > > >> > > >> > > >>>>>> >Requirements: > > >> > > >> > > >>>>>> > > > >> > > >> > > >>>>>> > - Plugins must be able to declare dependencies > > >> > > >> > > >>>>>> > - Plugins must be able to delay onDeviceReady() > > >> > > >> > > >>>>>> > - Plugins must be able to run code to initialize > > >> > > >> > > >>>>>> > > > >> > > >> > > >>>>>> > > > >> > > >> > > >>>>>> >Implementation modulemapper.js: > > >> > > >> > > >>>>>> > > > >> > > >> > > >>>>>> > - clobbers(...) > > >> > > >> > > >>>>>> > - merges(...) > > >> > > >> > > >>>>>> > - defaults(...) > > >> > > >> > > >>>>>> > - mapModules(wnd) > > >> > > >> > > >>>>>> > - getOriginalSymbol('FileSystem') > > >> > > >> > > >>>>>> > > > >> > > >> > > >>>>>> > > > >> > > >> > > >>>>>> >Start-up flow: > > >> > > >> > > >>>>>> > > > >> > > >> > > >>>>>> > 1. Parse all modules > > >> > > >> > > >>>>>> > 2. common-bootstrap: > > >> > > >> > > >>>>>> > 1. Loads list of modules named > > >>"cordova.*/symbols" > > >> > > >> > > >>>>>> > 2. Run modulemapper.mapModules(window); > > >> > > >> > > >>>>>> > 3. Loads list of modules named > "cordova.*/main" > > >> > > >> > > >>>>>> > > > >> > > >> > > >>>>>> > > > >> > > >> > > >>>>>> >symbols.js files: > > >> > > >> > > >>>>>> > > > >> > > >> > > >>>>>> > - Will make calls to modulemapper instead of > > >> exporting > > >> > > >> > > >>>>>>{clobbers:} > > >> > > >> > > >>>>>> > - This make dependencies work by require()ing > > >> > > >>dependent > > >> > > >> > > >>>>>>symbols > > >> > > >> > > >>>>>> > - We want the to be an evaluated .js file > instead > > >>of > > >> > > >> something > > >> > > >> > > >>>>>> listed > > >> > > >> > > >>>>>> >in > > >> > > >> > > >>>>>> > plugin.xml > > >> > > >> > > >>>>>> > - So that it can export based on browser > > >>version > > >> > > >> > > >>>>>> > > > >> > > >> > > >>>>>> > > > >> > > >> > > >>>>>> >Implementation Steps > > >> > > >> > > >>>>>> > > > >> > > >> > > >>>>>> > 1. Expose list of registered modules in > > >> > > >>scripts/require.js > > >> > > >> so > > >> > > >> > > >>>>>>that > > >> > > >> > > >>>>>> we > > >> > > >> > > >>>>>> > can loop over them > > >> > > >> > > >>>>>> > 2. Write modulemapper.js (and have unit tests, > of > > >> > course) > > >> > > >> > > >>>>>> > 3. Add logic to bootstrap.js that calls into > > >> > modulemapper > > >> > > >> > > >>>>>> > 4. create $PLUGIN_NAME/symbols.js files for each > > >> plugin > > >> > > >> within > > >> > > >> > > >>>>>> cordova. > > >> > > >> > > >>>>>> > 5. Add logic to bootstrap.js that calls into > > >> > modulemapper > > >> > > >> > > >>>>>> > 6. Create main.js files for those that currently > > >>have > > >> > > >>logic > > >> > > >> in > > >> > > >> > > >>>>>> their > > >> > > >> > > >>>>>> > platform.js files > > >> > > >> > > >>>>>> > > > >> > > >> > > >>>>>> >* > > >> > > >> > > >>>>>> > > >> > > >> > > >>>>>> > > >> > > >> > > >>>>> > > >> > > >> > > >>>> > > >> > > >> > > >>> > > >> > > >> > > >> > > >> > > >> > > > > >> > > >> > > > > >> > > >> > > > >> > > >> > > >> > > > > >> > > > > >> > > > >> > > > > >
