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 Wed, 20 Feb 2013 21:29:01 GMT
Agree with this. Sending out testing requests is fine, invariably not
everyone has a device for every platform. That said, this does not
exonerate anyone from not testing.

So in terms of the symbol mapping stuff, my question still stands: did you
test it on a device? If so, which device? Based on your answers we can
determine which platform/devices we are left to test, and from there we
can delegate responsibility for that to the right committers.

On 2/20/13 1:23 PM, "Jesse" <purplecabbage@gmail.com> wrote:

>Knowing the implication of your changes is pretty critical.
>IMHO everyone who commits to the generic portions of cordova-js should be
>prepared to at least smoke test in WP7or8, Android, iOS, and BB.  Or at a
>bare minimum have extreme confidence that your change will not affect
>them.
>
>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
>> > > >>>>>> >
>> > > >>>>>> >*
>> > > >>>>>>
>> > > >>>>>>
>> > > >>>>>
>> > > >>>>
>> > > >>>
>> > > >>
>> > >
>> > >
>> >
>>
>
>
>
>-- 
>@purplecabbage
>risingj.com


Mime
View raw message