cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michal Mocny <mmo...@chromium.org>
Subject Re: Symbol Mapping in Cordova
Date Wed, 20 Feb 2013 21:32:41 GMT
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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message