cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Grieve <agri...@chromium.org>
Subject Re: Symbol Mapping in Cordova
Date Wed, 20 Feb 2013 21:07:01 GMT
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