That looks great to me. When this was just WP7+8 it was borderline whether
the special case was simpler, but I'd much rather turn that special case
code into an extra chunk of metadata now that there are four or more
platforms that need special handling.

The more fiddly special cases we can eliminate the better.

Braden


On Thu, Nov 14, 2013 at 2:59 PM, Jesse <purplecabb...@gmail.com> wrote:

> I agree with this approach.  Non-existence of 'subdirectory' means the root
> is the root.
>
> platforms.js
>
> module.exports = {
>     'ios' : {
>         parser : './src/metadata/ios_parser',
>         url    : '
> https://git-wip-us.apache.org/repos/asf?p=cordova-ios.git',
>         version: '3.2.0-rc1'
>     },
>     'android' : {
>         parser : './src/metadata/android_parser',
>         url    :
> 'https://git-wip-us.apache.org/repos/asf?p=cordova-android.git',
>         version: '3.2.0-rc1'
>     },
>     'wp7' : {
>         parser : './src/metadata/wp7_parser',
>         url    : '
> https://git-wip-us.apache.org/repos/asf?p=cordova-wp8.git',
>         version: '3.2.0-rc1',
>
>         subdirectory: 'wp7'
>
>     },
>     'wp8' : {
>         parser : './src/metadata/wp8_parser',
>         url : 'https://git-wip-us.apache.org/repos/asf?p=cordova-wp8.git',
>         version: '3.2.0-rc1',
>
>         subdirectory: 'wp8'
>
>     },
> ...
>
>
>
> @purplecabbage
> risingj.com
>
>
> On Thu, Nov 14, 2013 at 11:48 AM, Maxime LUCE <max...@somatic.fr> wrote:
>
> > Hi,
> >
> > I fixed tests issues in https://github.com/apache/cordova-cli/pull/69.
> > I also added blackberry10 to platforms detection.
> >
> > This is a quick fix to make cli work again for those projects.
> > The best solution I think is to add a 'subdirectory' field in
> platforms.js
> > for any platform which is hosted in a sub directory into its
> repositories.
> > What do you think ?
> >
> > If you are ok with this, I can try a little fix.
> >
> > Cordialement.
> > ----------------------------
> > Maxime LUCE
> > max...@touchit.fr
> > 06 28 60 72 34
> > http://touchit.fr
> >
> > -----Original Message-----
> > From: Sergey Grebnov (Akvelon) [mailto:v-seg...@microsoft.com]
> > Sent: mercredi 6 novembre 2013 23:33
> > To: dev@cordova.apache.org
> > Subject: RE: Platforms which are in subdirectory in repositories can't be
> > added by cli since 3.1.0-0.2.0
> >
> > Just a note, I've sent similar fix for this issue as per of CB-5183 WP7/8
> > lib path is not correctly resolved by CLI. (a day ago)
> > https://github.com/apache/cordova-cli/pull/68
> >
> > I've reviewed Maxime's changes and they  look good for me (except
> > windows81 part which probably should be delayed) so I'm totally ok with
> > merging any PR (my or Maxime).
> >
> > PS.
> > There was previously similar path rsolving logic in src/platform.js which
> > was removed as per the following changes
> >
> https://github.com/apache/cordova-cli/commit/d75c0585c88ee9283db400f0c946027bd9d6e1ea
> >
> > -            var bin = path.join(cordova_util.libDirectory, target, id,
> > version, 'bin', 'create');
> > -            if(target == 'wp7') {
> > -                bin = path.join(cordova_util.libDirectory, 'wp', id,
> > version, 'wp7', 'bin', 'create');
> > -            }
> > -            else if(target == 'wp8') {
> > -                bin = path.join(cordova_util.libDirectory, 'wp', id,
> > version, 'wp8', 'bin', 'create');
> > -            }
> > -            else if(target == 'windows8') {
> > -                bin = path.join(cordova_util.libDirectory, 'windows8',
> > id, version, 'windows8', 'bin', 'create');
> > -            }
> >
> > Thx!
> > Sergey
> > -----Original Message-----
> > From: Jesse [mailto:purplecabb...@gmail.com]
> > Sent: Wednesday, November 6, 2013 11:18 PM
> > To: dev@cordova.apache.org
> > Subject: Re: Platforms which are in subdirectory in repositories can't be
> > added by cli since 3.1.0-0.2.0
> >
> > A few things:
> >
> > I cannot find a record of a signed CLA for Maxime Luce.
> > I expect to see Maxime listed here:
> > https://people.apache.org/committer-index.html
> >
> > The Windows 8.1 stuff should not be pulled in it's current state. 8.1
> > should not be a new platform, but an update to 8.0. I plan to do this for
> > 3.3.0
> >
> > The subfolder changes to the cli should not be required, this was all
> > working, but there may have been a regression at some point, or something
> > is out of sync between the published version of the platforms and the
> cli.
> >
> >
> >
> >
> >
> >
> >
> > @purplecabbage
> > risingj.com
> >
> >
> > On Wed, Nov 6, 2013 at 11:02 AM, Braden Shepherdson <bra...@chromium.org
> > >wrote:
> >
> > > There is existing special handling for blackberry10 scattered
> > > throughout the lazy-loading code. This will probably conflict with
> > > Maxime's changes and end in a path ending
> > > .../blackberry10/blackberry10. I'm not certain though, testing is
> > required.
> > >
> > > I've attached the log of the failed tests.
> > >
> > > Braden
> > >
> > >
> > > On Wed, Nov 6, 2013 at 1:50 PM, Maxime LUCE <max...@somatic.fr> wrote:
> > >
> > > > Braden,
> > > >
> > > > I comment on PR too,
> > > > I do not have test failure on my computer, which test fails on your
> > > > computer ?
> > > >
> > > > Cordialement.
> > > > ----------------------------
> > > > Maxime LUCE
> > > > max...@touchit.fr
> > > > 06 28 60 72 34
> > > > http://touchit.fr
> > > >
> > > > -----Original Message-----
> > > > From: bra...@google.com [mailto:bra...@google.com] On Behalf Of
> > > > Braden Shepherdson
> > > > Sent: mercredi 6 novembre 2013 19:22
> > > > To: dev@cordova.apache.org
> > > > Subject: Re: Platforms which are in subdirectory in repositories
> > > > can't be added by cli since 3.1.0-0.2.0
> > > >
> > > > Commented on the PR.
> > > >
> > > >
> > > > On Wed, Nov 6, 2013 at 1:20 PM, Maxime LUCE <max...@somatic.fr>
> wrote:
> > > >
> > > > > Hello,
> > > > >
> > > > > I found that platforms which are in a subdirectory of their
> > > > > repository can't be added by cli.
> > > > > I found that in the commit
> > > > > d75c0585c88ee9283db400f0c946027bd9d6e1ea in cordova-cli repository,
> > someone removed detection from these platforms.
> > > > >
> > > > > I created and resolved issue CB-5295 by doing a simple test over
> > > > > targeted platform.
> > > > > I think we must configure a "subdir" options in platforms.js to
> > > > > tell other modules that project can be in a sub directory in its
> > repository.
> > > > > What about that ? Can someone review and merge CB5295 ?
> > > > >
> > > > > You can find fix at :
> > > > > https://github.com/apache/cordova-cli/pull/69
> > > > >
> > > > > Cordialement.
> > > > > ----------------------------
> > > > > Maxime LUCE
> > > > > max...@touchit.fr
> > > > > 06 28 60 72 34
> > > > > http://touchit.fr
> > > > >
> > > > >
> > > > >
> > > >
> > >
> >
>

Reply via email to