You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@cordova.apache.org by Mark Koudritsky <ka...@gmail.com> on 2013/12/12 22:27:04 UTC

Re: Review Request 15775: Add --src & --link to "cordova create"

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15775/
-----------------------------------------------------------

(Updated Dec. 12, 2013, 9:27 p.m.)


Review request for cordova.


Changes
-------

--link was a boolean before, now it accepts a path, so it's either --src=path or --link=path.
Added a check in config_parser to make sure we are looking at an xml file that looks like Cordova config.xml to avoid overwriting some unrelated config.xml when symlinking to www dir.

Same diff on github: https://github.com/kamrik/cordova-cli/compare/src_www


Bugs: CB-4153
    https://issues.apache.org/jira/browse/CB-4153


Repository: cordova-cli


Description
-------

Add --src & --link to "cordova create".

Both --src and --source are accepted.
The path is passed via the lib.www.uri of the configuration object
that is later written to .cordova/config.json.
    
Cordova will keep going into child www folder(s) of the provided path.


Diffs (updated)
-----

  doc/help.txt 3361dd3 
  e2e/create.spec.js 560f82a 
  spec/metadata/android_parser.spec.js 52fc78a 
  spec/metadata/ios_parser.spec.js b56ddce 
  spec/metadata/windows8_parser.spec.js 5117148 
  spec/metadata/wp7_parser.spec.js 6dc0695 
  spec/metadata/wp8_parser.spec.js 8cd1923 
  src/cli.js 2916062 
  src/config_parser.js b6293d8 
  src/create.js 961d787 
  src/metadata/wp8_parser.js 163d56b 

Diff: https://reviews.apache.org/r/15775/diff/


Testing
-------

npm test;
cordova -d  create TstApp --source=some/www --link
a bunch of other cordova commands in the app


Thanks,

Mark Koudritsky

Re: Review Request 15775: Add --src & --link to "cordova create"

Posted by Andrew Grieve <ag...@chromium.org>.

Just two nits from me then I think it's good to go!

Let me know if you'd like someone else to look at it, or if I should merge
it in.

Nice work! This will be an appreciated feature I think :)


On Fri, Dec 20, 2013 at 1:43 PM, Andrew Grieve <ag...@chromium.org> wrote:

>    This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15775/
>    src/config_parser.js<https://reviews.apache.org/r/15775/diff/6/?file=401685#file401685line33> (Diff
> revision 6)
>
> function config_parser(path) {
>
>    33
>
>         throw new Error("This file does not seem to be a cordova config.xml file: " + path);
>
>   This seems unrelated to the goal of this change.
>
>
>    src/create.js<https://reviews.apache.org/r/15775/diff/6/?file=401686#file401686line126> (Diff
> revision 6)
>
> var sanedircontents = function (d) {
>
>    125
>
>         var www_id = config_json.lib.www.id || ('random_id_' + (Math.random() * 1e15).toString().substring(0,6));
>
>   we shouldn't need to give it a random ID, should we?
>
> Can you add a TODO about getting rid of the need for it?
>
>
> - Andrew Grieve
>
> On December 20th, 2013, 4:51 p.m. UTC, Mark Koudritsky wrote:
>   Review request for cordova.
> By Mark Koudritsky.
>
> *Updated Dec. 20, 2013, 4:51 p.m.*
>  *Bugs: * CB-4153 <https://issues.apache.org/jira/browse/CB-4153>
>  *Repository: * cordova-cli
> Description
>
> Add --src & --link to "cordova create".
>
> Both --src and --source are accepted.
> The path is passed via the lib.www.uri of the configuration object
> that is later written to .cordova/config.json.
>
> Cordova will keep going into child www folder(s) of the provided path.
>
>   Testing
>
> npm test;
> cordova -d  create TstApp --source=some/www --link
> a bunch of other cordova commands in the app
>
>   Diffs
>
>    - doc/help.txt (3361dd3)
>    - e2e/create.spec.js (560f82a)
>    - spec/cli.spec.js (215c64c)
>    - spec/metadata/android_parser.spec.js (52fc78a)
>    - spec/metadata/ios_parser.spec.js (b56ddce)
>    - spec/metadata/windows8_parser.spec.js (5117148)
>    - spec/metadata/wp7_parser.spec.js (6dc0695)
>    - spec/metadata/wp8_parser.spec.js (8cd1923)
>    - src/cli.js (2916062)
>    - src/config_parser.js (b6293d8)
>    - src/create.js (f741e2f)
>    - src/metadata/wp8_parser.js (163d56b)
>
> View Diff <https://reviews.apache.org/r/15775/diff/>
>