cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian LeRoux...@brian.io>
Subject Re: Symbol Mapping in Cordova
Date Wed, 20 Feb 2013 21:50:26 GMT
Agree this sort of thing should live in Jira as sub tasks for ppl to
test. (And yup the CI should help in the future.)

On Wed, Feb 20, 2013 at 1: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