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 >> > > >> > > >>>>>> > >> > > >> > > >>>>>> >* >> > > >> > > >>>>>> >> > > >> > > >>>>>> >> > > >> > > >>>>> >> > > >> > > >>>> >> > > >> > > >>> >> > > >> > > >> >> > > >> > > >> > > >> > > >> > > >> > >> > > >> >> > > >> > > >> > >>
