Mailing-List: contact dev-help@cordova.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cordova.apache.org
Received-SPF: pass (athena.apache.org: domain of mikeywbrooks@gmail.com
 designates 74.125.82.181 as permitted sender)
MIME-Version: 1.0
Sender: mikeywbrooks@gmail.com
In-Reply-To: 
 <CABiQX1Vx8Uj-=o4Vi+X3qp+=O3MXq1OFsg2auVki-3gHaVR=Mw@mail.gmail.com>
References: 
 <CAP7NMPpx1E8jPMWfoCThMUA7FxRjZkHT5_K3ay6PfALjppJrKw@mail.gmail.com>
	<CD4A8037.183A9%fil@adobe.com>
	<CAEeF2TfW=Z7miLM0gx9e-TUk=BvNUu2TgQAw=T_fRH08siaKEw@mail.gmail.com>
	<CABiQX1Vx8Uj-=o4Vi+X3qp+=O3MXq1OFsg2auVki-3gHaVR=Mw@mail.gmail.com>
Date: Wed, 20 Feb 2013 19:52:50 -0800
Message-ID: 
 <CAP7NMPoDbKctLiDRGHikn5Gnp0H3ASZNiytm=UUaQwfc=nV-sQ@mail.gmail.com>
Subject: Re: Symbol Mapping in Cordova
From: Michael Brooks <michael@michaelbrooks.ca>
To: dev@cordova.apache.org
Content-Type: multipart/alternative; boundary=f46d043be13038056904d63402f9

--f46d043be13038056904d63402f9
Content-Type: text/plain; charset=ISO-8859-1

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 <agrieve@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 <mmocny@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 <fil@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" <michael@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 <agrieve@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
> > > >><michael@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 <fil@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" <agrieve@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
> > > >><agrieve@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
> > > >> > > >><agrieve@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
> > > >> > > >>><agrieve@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
> > > >> > > >>>><agrieve@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 <fil@adobe.com
> >
> > > >> wrote:
> > > >> > > >>>>>
> > > >> > > >>>>>> This all seems reasonable. Shall we start a branch?
> > > >> > > >>>>>>
> > > >> > > >>>>>> On 1/15/13 2:47 PM, "Andrew Grieve" <agrieve@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
> > > >> > > >>>>>> >
> > > >> > > >>>>>> >*
> > > >> > > >>>>>>
> > > >> > > >>>>>>
> > > >> > > >>>>>
> > > >> > > >>>>
> > > >> > > >>>
> > > >> > > >>
> > > >> > >
> > > >> > >
> > > >> >
> > > >>
> > >
> > >
> >
>

--f46d043be13038056904d63402f9--