You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by Maxime LUCE <Ma...@somatic.fr> on 2013/11/27 12:21:32 UTC

Platforms in subdirectory into repository

In order to improve support of platforms in subdirectory, 

I created an improvement issue on JIRA : https://issues.apache.org/jira/browse/CB-5392
Then I created a fix available here : https://github.com/apache/cordova-cli/pull/91

Do you agree with that way ?


Cordialement.
----------------------------
Maxime LUCE
maxime@touchit.fr
06 28 60 72 34
http://touchit.fr

-----Original Message-----
From: braden@google.com [mailto:braden@google.com] On Behalf Of Braden Shepherdson
Sent: jeudi 14 novembre 2013 21:03
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

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 <pu...@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 <Ma...@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
> > maxime@touchit.fr
> > 06 28 60 72 34
> > http://touchit.fr
> >
> > -----Original Message-----
> > From: Sergey Grebnov (Akvelon) [mailto:v-segreb@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/d75c0585c88ee9283db400f0c
> 946027bd9d6e1ea
> >
> > -            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:purplecabbage@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 
> > <braden@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 <Ma...@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
> > > > maxime@touchit.fr
> > > > 06 28 60 72 34
> > > > http://touchit.fr
> > > >
> > > > -----Original Message-----
> > > > From: braden@google.com [mailto:braden@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 <Ma...@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
> > > > > maxime@touchit.fr
> > > > > 06 28 60 72 34
> > > > > http://touchit.fr
> > > > >
> > > > >
> > > > >
> > > >
> > >
> >
>

RE: Platforms in subdirectory into repository

Posted by Maxime LUCE <Ma...@somatic.fr>.
You can run each test separately by doing in console (in your cordova-cli root)

node node_modules\jasmine-node\bin\jasmine-node spec\build.spec.js

Tests which fail on Windows are located in hook.spec.js if I remember correctly.

Cordialement.
----------------------------
Maxime LUCE
maxime@touchit.fr
06 28 60 72 34
http://touchit.fr

-----Original Message-----
From: purplecabbage [mailto:purplecabbage@gmail.com] 
Sent: mercredi 27 novembre 2013 18:17
To: dev@cordova.apache.org
Subject: Re: Platforms in subdirectory into repository

I am currently testing it. 
Tests are currently f'd on windows, so it is very difficult to both develop and test as you require both os's. 
I am looking at fixing the way the tests are run so we can simplify this. 

Sent from my iPhone

> On Nov 27, 2013, at 7:31 AM, Maxime LUCE <Ma...@somatic.fr> wrote:
> 
> I didn't test it specifically but I take it into account in my development.
> I removed the specific test for blackberry as it is standardized in lazy_load.js, you can see it at :
> https://github.com/apache/cordova-cli/pull/91/files#diff-a97c0300b0c3e
> 72467e54b438343f607L102
> 
> 
> Cordialement.
> ----------------------------
> Maxime LUCE
> maxime@touchit.fr
> 06 28 60 72 34
> http://touchit.fr
> 
> -----Original Message-----
> From: bryan@bryanhiggins.net [mailto:bryan@bryanhiggins.net] On Behalf 
> Of Bryan Higgins
> Sent: mercredi 27 novembre 2013 15:25
> To: dev@cordova.apache.org
> Subject: Re: Platforms in subdirectory into repository
> 
> I like the idea of standardizing it this way. Was this patch tested on bb10?
> 
> 
>> On Wed, Nov 27, 2013 at 6:21 AM, Maxime LUCE <Ma...@somatic.fr> wrote:
>> 
>> In order to improve support of platforms in subdirectory,
>> 
>> I created an improvement issue on JIRA :
>> https://issues.apache.org/jira/browse/CB-5392
>> Then I created a fix available here :
>> https://github.com/apache/cordova-cli/pull/91
>> 
>> Do you agree with that way ?
>> 
>> 
>> Cordialement.
>> ----------------------------
>> Maxime LUCE
>> maxime@touchit.fr
>> 06 28 60 72 34
>> http://touchit.fr
>> 
>> -----Original Message-----
>> From: braden@google.com [mailto:braden@google.com] On Behalf Of 
>> Braden Shepherdson
>> Sent: jeudi 14 novembre 2013 21:03
>> 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
>> 
>> 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 <pu...@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 <Ma...@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
>>>> maxime@touchit.fr
>>>> 06 28 60 72 34
>>>> http://touchit.fr
>>>> 
>>>> -----Original Message-----
>>>> From: Sergey Grebnov (Akvelon) [mailto:v-segreb@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/d75c0585c88ee9283db400f
>>> 0c
>>> 946027bd9d6e1ea
>>>> 
>>>> -            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:purplecabbage@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 
>>>> <braden@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 <Ma...@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
>>>>>> maxime@touchit.fr
>>>>>> 06 28 60 72 34
>>>>>> http://touchit.fr
>>>>>> 
>>>>>> -----Original Message-----
>>>>>> From: braden@google.com [mailto:braden@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 
>>>>>> <Ma...@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 maxime@touchit.fr
>>>>>>> 06 28 60 72 34
>>>>>>> http://touchit.fr
>> 

Re: Platforms in subdirectory into repository

Posted by purplecabbage <pu...@gmail.com>.
I am currently testing it. 
Tests are currently f'd on windows, so it is very difficult to both develop and test as you require both os's. 
I am looking at fixing the way the tests are run so we can simplify this. 

Sent from my iPhone

> On Nov 27, 2013, at 7:31 AM, Maxime LUCE <Ma...@somatic.fr> wrote:
> 
> I didn't test it specifically but I take it into account in my development.
> I removed the specific test for blackberry as it is standardized in lazy_load.js, you can see it at :
> https://github.com/apache/cordova-cli/pull/91/files#diff-a97c0300b0c3e72467e54b438343f607L102
> 
> 
> Cordialement.
> ----------------------------
> Maxime LUCE
> maxime@touchit.fr
> 06 28 60 72 34
> http://touchit.fr
> 
> -----Original Message-----
> From: bryan@bryanhiggins.net [mailto:bryan@bryanhiggins.net] On Behalf Of Bryan Higgins
> Sent: mercredi 27 novembre 2013 15:25
> To: dev@cordova.apache.org
> Subject: Re: Platforms in subdirectory into repository
> 
> I like the idea of standardizing it this way. Was this patch tested on bb10?
> 
> 
>> On Wed, Nov 27, 2013 at 6:21 AM, Maxime LUCE <Ma...@somatic.fr> wrote:
>> 
>> In order to improve support of platforms in subdirectory,
>> 
>> I created an improvement issue on JIRA :
>> https://issues.apache.org/jira/browse/CB-5392
>> Then I created a fix available here :
>> https://github.com/apache/cordova-cli/pull/91
>> 
>> Do you agree with that way ?
>> 
>> 
>> Cordialement.
>> ----------------------------
>> Maxime LUCE
>> maxime@touchit.fr
>> 06 28 60 72 34
>> http://touchit.fr
>> 
>> -----Original Message-----
>> From: braden@google.com [mailto:braden@google.com] On Behalf Of Braden 
>> Shepherdson
>> Sent: jeudi 14 novembre 2013 21:03
>> 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
>> 
>> 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 <pu...@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 <Ma...@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
>>>> maxime@touchit.fr
>>>> 06 28 60 72 34
>>>> http://touchit.fr
>>>> 
>>>> -----Original Message-----
>>>> From: Sergey Grebnov (Akvelon) [mailto:v-segreb@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/d75c0585c88ee9283db400f
>>> 0c
>>> 946027bd9d6e1ea
>>>> 
>>>> -            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:purplecabbage@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 
>>>> <braden@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 <Ma...@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
>>>>>> maxime@touchit.fr
>>>>>> 06 28 60 72 34
>>>>>> http://touchit.fr
>>>>>> 
>>>>>> -----Original Message-----
>>>>>> From: braden@google.com [mailto:braden@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 
>>>>>> <Ma...@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 maxime@touchit.fr
>>>>>>> 06 28 60 72 34
>>>>>>> http://touchit.fr
>> 

RE: Platforms in subdirectory into repository

Posted by Maxime LUCE <Ma...@somatic.fr>.
I didn't test it specifically but I take it into account in my development.
I removed the specific test for blackberry as it is standardized in lazy_load.js, you can see it at :
https://github.com/apache/cordova-cli/pull/91/files#diff-a97c0300b0c3e72467e54b438343f607L102


Cordialement.
----------------------------
Maxime LUCE
maxime@touchit.fr
06 28 60 72 34
http://touchit.fr

-----Original Message-----
From: bryan@bryanhiggins.net [mailto:bryan@bryanhiggins.net] On Behalf Of Bryan Higgins
Sent: mercredi 27 novembre 2013 15:25
To: dev@cordova.apache.org
Subject: Re: Platforms in subdirectory into repository

I like the idea of standardizing it this way. Was this patch tested on bb10?


On Wed, Nov 27, 2013 at 6:21 AM, Maxime LUCE <Ma...@somatic.fr> wrote:

> In order to improve support of platforms in subdirectory,
>
> I created an improvement issue on JIRA :
> https://issues.apache.org/jira/browse/CB-5392
> Then I created a fix available here :
> https://github.com/apache/cordova-cli/pull/91
>
> Do you agree with that way ?
>
>
> Cordialement.
> ----------------------------
> Maxime LUCE
> maxime@touchit.fr
> 06 28 60 72 34
> http://touchit.fr
>
> -----Original Message-----
> From: braden@google.com [mailto:braden@google.com] On Behalf Of Braden 
> Shepherdson
> Sent: jeudi 14 novembre 2013 21:03
> 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
>
> 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 <pu...@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 <Ma...@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
> > > maxime@touchit.fr
> > > 06 28 60 72 34
> > > http://touchit.fr
> > >
> > > -----Original Message-----
> > > From: Sergey Grebnov (Akvelon) [mailto:v-segreb@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/d75c0585c88ee9283db400f
> > 0c
> > 946027bd9d6e1ea
> > >
> > > -            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:purplecabbage@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 
> > > <braden@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 <Ma...@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
> > > > > maxime@touchit.fr
> > > > > 06 28 60 72 34
> > > > > http://touchit.fr
> > > > >
> > > > > -----Original Message-----
> > > > > From: braden@google.com [mailto:braden@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 
> > > > > <Ma...@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 maxime@touchit.fr
> > > > > > 06 28 60 72 34
> > > > > > http://touchit.fr
> > > > > >
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: Platforms in subdirectory into repository

Posted by Bryan Higgins <bh...@blackberry.com>.
I like the idea of standardizing it this way. Was this patch tested on bb10?


On Wed, Nov 27, 2013 at 6:21 AM, Maxime LUCE <Ma...@somatic.fr> wrote:

> In order to improve support of platforms in subdirectory,
>
> I created an improvement issue on JIRA :
> https://issues.apache.org/jira/browse/CB-5392
> Then I created a fix available here :
> https://github.com/apache/cordova-cli/pull/91
>
> Do you agree with that way ?
>
>
> Cordialement.
> ----------------------------
> Maxime LUCE
> maxime@touchit.fr
> 06 28 60 72 34
> http://touchit.fr
>
> -----Original Message-----
> From: braden@google.com [mailto:braden@google.com] On Behalf Of Braden
> Shepherdson
> Sent: jeudi 14 novembre 2013 21:03
> 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
>
> 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 <pu...@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 <Ma...@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
> > > maxime@touchit.fr
> > > 06 28 60 72 34
> > > http://touchit.fr
> > >
> > > -----Original Message-----
> > > From: Sergey Grebnov (Akvelon) [mailto:v-segreb@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/d75c0585c88ee9283db400f0c
> > 946027bd9d6e1ea
> > >
> > > -            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:purplecabbage@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
> > > <braden@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 <Ma...@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
> > > > > maxime@touchit.fr
> > > > > 06 28 60 72 34
> > > > > http://touchit.fr
> > > > >
> > > > > -----Original Message-----
> > > > > From: braden@google.com [mailto:braden@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 <Ma...@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
> > > > > > maxime@touchit.fr
> > > > > > 06 28 60 72 34
> > > > > > http://touchit.fr
> > > > > >
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>