cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Filip Maj <...@adobe.com>
Subject Re: Symbol Mapping in Cordova
Date Thu, 21 Feb 2013 19:08:33 GMT
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" <michael@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 <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
>> > > >> > > >>>>>> >
>> > > >> > > >>>>>> >*
>> > > >> > > >>>>>>
>> > > >> > > >>>>>>
>> > > >> > > >>>>>
>> > > >> > > >>>>
>> > > >> > > >>>
>> > > >> > > >>
>> > > >> > >
>> > > >> > >
>> > > >> >
>> > > >>
>> > >
>> > >
>> >
>>


Mime
View raw message