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/10/16 21:47:00 UTC
Review Request 14681: CB-4748: Fail quickly if dir passed to cordova create
is not empty.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14681/
-----------------------------------------------------------
Review request for cordova.
Repository: cordova-cli
Description
-------
CB-4748: Fail quickly if dir passed to cordova create is not empty.
Also add cfg parameter to create() for extra configuration which saved in .cordova/config.json
Should come together with the corresponding change in mca.js:
https://github.com/MobileChromeApps/mobile-chrome-apps/pull/25
Diffs
-----
src/create.js 4b2b950
Diff: https://reviews.apache.org/r/14681/diff/
Testing
-------
./mca.js create io.some.app2
Thanks,
Mark Koudritsky
Re: Review Request 14681: CB-4748: Fail quickly if dir passed to cordova
create is not empty.
Posted by Mark Koudritsky <ka...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14681/
-----------------------------------------------------------
(Updated Oct. 18, 2013, 6:27 p.m.)
Review request for cordova.
Changes
-------
Fixed the tests.
Bugs: CB-4748
https://issues.apache.org/jira/browse/CB-4748
Repository: cordova-cli
Description
-------
CB-4748: Fail quickly if dir passed to cordova create is not empty.
This change also exists as: https://github.com/apache/cordova-cli/pull/55
Also add cfg parameter to create() for extra configuration which saved in .cordova/config.json
Should come together with the corresponding change in mca.js:
https://github.com/MobileChromeApps/mobile-chrome-apps/pull/25
Diffs (updated)
-----
spec/create.spec.js e46569d
src/create.js 4b2b950
Diff: https://reviews.apache.org/r/14681/diff/
Testing (updated)
-------
./mca.js create io.some.app2
$ node node_modules/jasmine-node/bin/jasmine-node spec/create.spec.js
Finished in 0.176 seconds
10 tests, 43 assertions, 0 failures
$ npm test
Finished in 43.867 seconds
354 tests, 591 assertions, 10 failures
(The failures are in wp7 windows7 and firefoxos)
Thanks,
Mark Koudritsky
Re: Review Request 14681: CB-4748: Fail quickly if dir passed to cordova
create is not empty.
Posted by Mark Koudritsky <ka...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14681/
-----------------------------------------------------------
(Updated Oct. 17, 2013, 3:49 p.m.)
Review request for cordova.
Bugs: CB-4748
https://issues.apache.org/jira/browse/CB-4748
Repository: cordova-cli
Description
-------
CB-4748: Fail quickly if dir passed to cordova create is not empty.
This change also exists as: https://github.com/apache/cordova-cli/pull/55
Also add cfg parameter to create() for extra configuration which saved in .cordova/config.json
Should come together with the corresponding change in mca.js:
https://github.com/MobileChromeApps/mobile-chrome-apps/pull/25
Diffs (updated)
-----
src/create.js 4b2b950
Diff: https://reviews.apache.org/r/14681/diff/
Testing
-------
./mca.js create io.some.app2
Thanks,
Mark Koudritsky
Re: Review Request 14681: CB-4748: Fail quickly if dir passed to cordova
create is not empty.
Posted by Braden Shepherdson <br...@chromium.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14681/#review27124
-----------------------------------------------------------
src/create.js
<https://reviews.apache.org/r/14681/#comment52772>
typo: porject
- Braden Shepherdson
On Oct. 16, 2013, 9:27 p.m., Mark Koudritsky wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14681/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2013, 9:27 p.m.)
>
>
> Review request for cordova.
>
>
> Bugs: CB-4748
> https://issues.apache.org/jira/browse/CB-4748
>
>
> Repository: cordova-cli
>
>
> Description
> -------
>
> CB-4748: Fail quickly if dir passed to cordova create is not empty.
> This change also exists as: https://github.com/apache/cordova-cli/pull/55
>
>
> Also add cfg parameter to create() for extra configuration which saved in .cordova/config.json
> Should come together with the corresponding change in mca.js:
> https://github.com/MobileChromeApps/mobile-chrome-apps/pull/25
>
>
> Diffs
> -----
>
> src/create.js 4b2b950
>
> Diff: https://reviews.apache.org/r/14681/diff/
>
>
> Testing
> -------
>
> ./mca.js create io.some.app2
>
>
> Thanks,
>
> Mark Koudritsky
>
>
Re: Review Request 14681: CB-4748: Fail quickly if dir passed to cordova
create is not empty.
Posted by Braden Shepherdson <br...@chromium.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14681/#review27125
-----------------------------------------------------------
Ship it!
Ship It!
- Braden Shepherdson
On Oct. 16, 2013, 9:27 p.m., Mark Koudritsky wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14681/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2013, 9:27 p.m.)
>
>
> Review request for cordova.
>
>
> Bugs: CB-4748
> https://issues.apache.org/jira/browse/CB-4748
>
>
> Repository: cordova-cli
>
>
> Description
> -------
>
> CB-4748: Fail quickly if dir passed to cordova create is not empty.
> This change also exists as: https://github.com/apache/cordova-cli/pull/55
>
>
> Also add cfg parameter to create() for extra configuration which saved in .cordova/config.json
> Should come together with the corresponding change in mca.js:
> https://github.com/MobileChromeApps/mobile-chrome-apps/pull/25
>
>
> Diffs
> -----
>
> src/create.js 4b2b950
>
> Diff: https://reviews.apache.org/r/14681/diff/
>
>
> Testing
> -------
>
> ./mca.js create io.some.app2
>
>
> Thanks,
>
> Mark Koudritsky
>
>
Re: Review Request 14681: CB-4748: Fail quickly if dir passed to cordova
create is not empty.
Posted by Mark Koudritsky <ka...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14681/
-----------------------------------------------------------
(Updated Oct. 16, 2013, 9:27 p.m.)
Review request for cordova.
Changes
-------
Changed to use name and id from cfg if supplied.
like this:
id = id || cfg.id || DEFAULT_ID
Bugs: CB-4748
https://issues.apache.org/jira/browse/CB-4748
Repository: cordova-cli
Description
-------
CB-4748: Fail quickly if dir passed to cordova create is not empty.
This change also exists as: https://github.com/apache/cordova-cli/pull/55
Also add cfg parameter to create() for extra configuration which saved in .cordova/config.json
Should come together with the corresponding change in mca.js:
https://github.com/MobileChromeApps/mobile-chrome-apps/pull/25
Diffs (updated)
-----
src/create.js 4b2b950
Diff: https://reviews.apache.org/r/14681/diff/
Testing
-------
./mca.js create io.some.app2
Thanks,
Mark Koudritsky
Re: Review Request 14681: CB-4748: Fail quickly if dir passed to cordova
create is not empty.
Posted by Mark Koudritsky <ka...@gmail.com>.
> On Oct. 16, 2013, 8:45 p.m., Michal Mocny wrote:
> > src/create.js, line 76
> > <https://reviews.apache.org/r/14681/diff/1/?file=365324#file365324line76>
> >
> > Why bother supporting empty directories? If its a huge directory it may take a while to fail?
It's mostly for the sake of supporting . as the dir.
> mkdir tstapp
> cd tstapp
> cordova create .
I think some people would get annoyed if this failed complaining that . already exists.
> On Oct. 16, 2013, 8:45 p.m., Michal Mocny wrote:
> > src/create.js, line 55
> > <https://reviews.apache.org/r/14681/diff/1/?file=365324#file365324line55>
> >
> > what if cfg already has a 'name'?
> >
> > what would it mean to call create with a name argument and a name key on cfg? perhaps we just make that invalid?
One option is to use the argument if it's given and disregard whatever might be in cfg, but use cfg.name if the argument is a falsy value. What do you think about the following?
cfg = cfg || {};
id = id || cfg.id || DEFAULT_ID;
name = name || cfg.name || DEFAULT_NAME;
cfg.id = id;
cfg.name = name;
- Mark
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14681/#review27082
-----------------------------------------------------------
On Oct. 16, 2013, 8:17 p.m., Mark Koudritsky wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14681/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2013, 8:17 p.m.)
>
>
> Review request for cordova.
>
>
> Bugs: CB-4748
> https://issues.apache.org/jira/browse/CB-4748
>
>
> Repository: cordova-cli
>
>
> Description
> -------
>
> CB-4748: Fail quickly if dir passed to cordova create is not empty.
> This change also exists as: https://github.com/apache/cordova-cli/pull/55
>
>
> Also add cfg parameter to create() for extra configuration which saved in .cordova/config.json
> Should come together with the corresponding change in mca.js:
> https://github.com/MobileChromeApps/mobile-chrome-apps/pull/25
>
>
> Diffs
> -----
>
> src/create.js 4b2b950
>
> Diff: https://reviews.apache.org/r/14681/diff/
>
>
> Testing
> -------
>
> ./mca.js create io.some.app2
>
>
> Thanks,
>
> Mark Koudritsky
>
>
Re: Review Request 14681: CB-4748: Fail quickly if dir passed to cordova
create is not empty.
Posted by Michal Mocny <mm...@chromium.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14681/#review27082
-----------------------------------------------------------
src/create.js
<https://reviews.apache.org/r/14681/#comment52722>
what if cfg already has a 'name'?
what would it mean to call create with a name argument and a name key on cfg? perhaps we just make that invalid?
src/create.js
<https://reviews.apache.org/r/14681/#comment52724>
Why bother supporting empty directories? If its a huge directory it may take a while to fail?
- Michal Mocny
On Oct. 16, 2013, 8:17 p.m., Mark Koudritsky wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14681/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2013, 8:17 p.m.)
>
>
> Review request for cordova.
>
>
> Bugs: CB-4748
> https://issues.apache.org/jira/browse/CB-4748
>
>
> Repository: cordova-cli
>
>
> Description
> -------
>
> CB-4748: Fail quickly if dir passed to cordova create is not empty.
> This change also exists as: https://github.com/apache/cordova-cli/pull/55
>
>
> Also add cfg parameter to create() for extra configuration which saved in .cordova/config.json
> Should come together with the corresponding change in mca.js:
> https://github.com/MobileChromeApps/mobile-chrome-apps/pull/25
>
>
> Diffs
> -----
>
> src/create.js 4b2b950
>
> Diff: https://reviews.apache.org/r/14681/diff/
>
>
> Testing
> -------
>
> ./mca.js create io.some.app2
>
>
> Thanks,
>
> Mark Koudritsky
>
>
Re: Review Request 14681: CB-4748: Fail quickly if dir passed to cordova
create is not empty.
Posted by Mark Koudritsky <ka...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14681/
-----------------------------------------------------------
(Updated Oct. 16, 2013, 8:17 p.m.)
Review request for cordova.
Bugs: CB-4748
https://issues.apache.org/jira/browse/CB-4748
Repository: cordova-cli
Description
-------
CB-4748: Fail quickly if dir passed to cordova create is not empty.
This change also exists as: https://github.com/apache/cordova-cli/pull/55
Also add cfg parameter to create() for extra configuration which saved in .cordova/config.json
Should come together with the corresponding change in mca.js:
https://github.com/MobileChromeApps/mobile-chrome-apps/pull/25
Diffs
-----
src/create.js 4b2b950
Diff: https://reviews.apache.org/r/14681/diff/
Testing
-------
./mca.js create io.some.app2
Thanks,
Mark Koudritsky
Re: Review Request 14681: CB-4748: Fail quickly if dir passed to cordova
create is not empty.
Posted by Mark Koudritsky <ka...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14681/
-----------------------------------------------------------
(Updated Oct. 16, 2013, 7:56 p.m.)
Review request for cordova.
Repository: cordova-cli
Description (updated)
-------
CB-4748: Fail quickly if dir passed to cordova create is not empty.
This change also exists as: https://github.com/apache/cordova-cli/pull/55
Also add cfg parameter to create() for extra configuration which saved in .cordova/config.json
Should come together with the corresponding change in mca.js:
https://github.com/MobileChromeApps/mobile-chrome-apps/pull/25
Diffs
-----
src/create.js 4b2b950
Diff: https://reviews.apache.org/r/14681/diff/
Testing
-------
./mca.js create io.some.app2
Thanks,
Mark Koudritsky