You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by Michal Mocny <mm...@chromium.org> on 2013/11/27 22:51:08 UTC
Review Request 15889: [android] call out to platform check_req script
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15889/
-----------------------------------------------------------
Review request for cordova.
Repository: cordova-cli
Description
-------
.
Diffs
-----
src/metadata/android_parser.js 535d5b2
Diff: https://reviews.apache.org/r/15889/diff/
Testing
-------
Thanks,
Michal Mocny
Re: Review Request 15889: [android] call out to platform check_req script
Posted by Braden Shepherdson <br...@chromium.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15889/#review29517
-----------------------------------------------------------
src/metadata/android_parser.js
<https://reviews.apache.org/r/15889/#comment56830>
This is duplicating the logic in the lazy loader for checking for custom configs and so on.
The correct flow is:
lazy_load.based_on_config(project_root, 'android').then(function(lib_path) {
// shell out to lib_path/bin/check_reqs
});
- Braden Shepherdson
On Nov. 27, 2013, 9:51 p.m., Michal Mocny wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15889/
> -----------------------------------------------------------
>
> (Updated Nov. 27, 2013, 9:51 p.m.)
>
>
> Review request for cordova.
>
>
> Repository: cordova-cli
>
>
> Description
> -------
>
> .
>
>
> Diffs
> -----
>
> src/metadata/android_parser.js 535d5b2
>
> Diff: https://reviews.apache.org/r/15889/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michal Mocny
>
>
Re: Review Request 15889: [android] call out to platform check_req script
Posted by Andrew Grieve <ag...@chromium.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15889/#review29532
-----------------------------------------------------------
Ship it!
Ship It!
- Andrew Grieve
On Nov. 28, 2013, 2:53 p.m., Michal Mocny wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15889/
> -----------------------------------------------------------
>
> (Updated Nov. 28, 2013, 2:53 p.m.)
>
>
> Review request for cordova.
>
>
> Repository: cordova-cli
>
>
> Description
> -------
>
> .
>
>
> Diffs
> -----
>
> src/metadata/android_parser.js 535d5b2
>
> Diff: https://reviews.apache.org/r/15889/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michal Mocny
>
>
Re: Review Request 15889: [android] call out to platform check_req script
Posted by Mark Koudritsky <ka...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15889/#review29531
-----------------------------------------------------------
src/metadata/android_parser.js
<https://reviews.apache.org/r/15889/#comment56885>
Nit: 2 -> 4 space indent
runs ok on windows (tried cordova create; cordova platform add android).
- Mark Koudritsky
On Nov. 28, 2013, 2:53 p.m., Michal Mocny wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15889/
> -----------------------------------------------------------
>
> (Updated Nov. 28, 2013, 2:53 p.m.)
>
>
> Review request for cordova.
>
>
> Repository: cordova-cli
>
>
> Description
> -------
>
> .
>
>
> Diffs
> -----
>
> src/metadata/android_parser.js 535d5b2
>
> Diff: https://reviews.apache.org/r/15889/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michal Mocny
>
>
Re: Review Request 15889: [android] call out to platform check_req script
Posted by Michal Mocny <mm...@chromium.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15889/#review29530
-----------------------------------------------------------
Also, I have not tested in Windows, I'll make sure that happens asap.
- Michal Mocny
On Nov. 28, 2013, 2:53 p.m., Michal Mocny wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15889/
> -----------------------------------------------------------
>
> (Updated Nov. 28, 2013, 2:53 p.m.)
>
>
> Review request for cordova.
>
>
> Repository: cordova-cli
>
>
> Description
> -------
>
> .
>
>
> Diffs
> -----
>
> src/metadata/android_parser.js 535d5b2
>
> Diff: https://reviews.apache.org/r/15889/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michal Mocny
>
>
Re: Review Request 15889: [android] call out to platform check_req script
Posted by Michal Mocny <mm...@chromium.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15889/
-----------------------------------------------------------
(Updated Nov. 28, 2013, 2:53 p.m.)
Review request for cordova.
Changes
-------
New version: doesn't execute check_reqs as a child_process when its a node command anyway! Thanks to Bradens tip, this much cleaner. Likely we want to update the other platforms as well?
Repository: cordova-cli
Description
-------
.
Diffs (updated)
-----
src/metadata/android_parser.js 535d5b2
Diff: https://reviews.apache.org/r/15889/diff/
Testing
-------
Thanks,
Michal Mocny
Re: Review Request 15889: [android] call out to platform check_req script
Posted by Carlos Santana <cs...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15889/#review29528
-----------------------------------------------------------
Did you performed a test running on Windows?
Remember that CLI is not only for Linux and OSX, it also should support Windows.
- Carlos Santana
On Nov. 27, 2013, 9:51 p.m., Michal Mocny wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15889/
> -----------------------------------------------------------
>
> (Updated Nov. 27, 2013, 9:51 p.m.)
>
>
> Review request for cordova.
>
>
> Repository: cordova-cli
>
>
> Description
> -------
>
> .
>
>
> Diffs
> -----
>
> src/metadata/android_parser.js 535d5b2
>
> Diff: https://reviews.apache.org/r/15889/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michal Mocny
>
>
Re: Review Request 15889: [android] call out to platform check_req script
Posted by Joe Bowser <bo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15889/#review29514
-----------------------------------------------------------
I think that this should be shipped. However, someone who is more familiar with the CLI should look at this more closely, so I won't check the Ship It box. :P
- Joe Bowser
On Nov. 27, 2013, 9:51 p.m., Michal Mocny wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15889/
> -----------------------------------------------------------
>
> (Updated Nov. 27, 2013, 9:51 p.m.)
>
>
> Review request for cordova.
>
>
> Repository: cordova-cli
>
>
> Description
> -------
>
> .
>
>
> Diffs
> -----
>
> src/metadata/android_parser.js 535d5b2
>
> Diff: https://reviews.apache.org/r/15889/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michal Mocny
>
>
Re: Review Request 15889: [android] call out to platform check_req script
Posted by Michal Mocny <mm...@chromium.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15889/#review29518
-----------------------------------------------------------
src/metadata/android_parser.js
<https://reviews.apache.org/r/15889/#comment56832>
Thats awesome. Will make the change. Should apply to other platforms as well.
src/metadata/android_parser.js
<https://reviews.apache.org/r/15889/#comment56837>
Probably also I should not be execiting the script when I can just call the promise-based node command..
- Michal Mocny
On Nov. 27, 2013, 9:51 p.m., Michal Mocny wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15889/
> -----------------------------------------------------------
>
> (Updated Nov. 27, 2013, 9:51 p.m.)
>
>
> Review request for cordova.
>
>
> Repository: cordova-cli
>
>
> Description
> -------
>
> .
>
>
> Diffs
> -----
>
> src/metadata/android_parser.js 535d5b2
>
> Diff: https://reviews.apache.org/r/15889/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michal Mocny
>
>