You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by jsoref <gi...@git.apache.org> on 2014/08/21 01:45:21 UTC
[GitHub] cordova-mobile-spec pull request: CB-7350 Make createmobilespec fr...
GitHub user jsoref opened a pull request:
https://github.com/apache/cordova-mobile-spec/pull/103
CB-7350 Make createmobilespec friendlier
Unlike the other pull requests of late, I do plan to squash these (and write a better commit message).
They're split up here for review... The best way to review them is:
```sh
git log --oneline --decorate -w -p master..FETCH_HEAD
```
apache/cordova-js#78 will help with one of the other items (cordova-js has grown some dependencies, and users aren't likely to have run npm install)
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/jsoref/cordova-mobile-spec docwork
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-mobile-spec/pull/103.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #103
----
commit e0cf43f6ef07f9fadc3c5fb33284de5963324f80
Author: Josh Soref <js...@blackberry.com>
Date: 2014-08-20T21:08:40Z
Improve README.md markdown
Also annotate points where I think the content is wrong
commit fbee67619dd44696203f61d9a299c3b283c023a0
Author: Josh Soref <js...@blackberry.com>
Date: 2014-08-20T21:09:00Z
Remove trailing whitespace
commit 8abdb4ec1c98c674b18fecdac91e7a1022a7da27
Author: Josh Soref <js...@blackberry.com>
Date: 2014-08-20T21:09:30Z
Correct __dirname referenced paths
commit 494632fed76f146122e2c1f62edb9d99df607d6d
Author: Josh Soref <js...@blackberry.com>
Date: 2014-08-20T21:10:13Z
Comment out nonsense claim about consumers
Most consumers of this are testers, not developers
commit e281014b4614da42404e56d0d09c5bc1d01ef9ba
Author: Josh Soref <js...@blackberry.com>
Date: 2014-08-20T21:10:45Z
Explain how to get coho
commit 11498c9cf8ac80c3e2222bd75b6cffe2e107ed84
Author: Josh Soref <js...@blackberry.com>
Date: 2014-08-20T21:11:10Z
Fix instructions for getting cli+lib and plugins
commit 4dc85564ca3b9a524965ec432173f52e8a788503
Author: Josh Soref <js...@blackberry.com>
Date: 2014-08-20T22:08:52Z
Add quietshell pushd/popd
commit 423d36e44cc7c9f7b84dc9b0d31b435929b56f67
Author: Josh Soref <js...@blackberry.com>
Date: 2014-08-20T22:23:51Z
support debug logging level
commit 851241d6a5c21ba16ff31271ddc32d04376412d9
Author: Josh Soref <js...@blackberry.com>
Date: 2014-08-20T22:31:32Z
quit instead of return in top level code...
commit 6536d2225ea67f2cc608a5d9a9777fca43bd802b
Author: Josh Soref <js...@blackberry.com>
Date: 2014-08-20T23:08:08Z
Skip broken platforms
commit c5baa3978a0ab964101250d4aacd15551f8b7e87
Author: Josh Soref <js...@blackberry.com>
Date: 2014-08-20T23:12:42Z
Refactor installPlugins, updateJS, summary
Only call them if there are platforms left
commit 54f0b41632e5f4b073aaf012a0d19110dac95434
Author: Josh Soref <js...@blackberry.com>
Date: 2014-08-20T23:17:57Z
Add could not find routine with coho instructions
commit 7569a98a4bc4ec6cdd8951226f5ffb7ab283d02d
Author: Josh Soref <js...@blackberry.com>
Date: 2014-08-20T23:20:55Z
Require grunt-cli
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-mobile-spec pull request: CB-7350 Make createmobilespec fr...
Posted by jengee <gi...@git.apache.org>.
Github user jengee commented on a diff in the pull request:
https://github.com/apache/cordova-mobile-spec/pull/103#discussion_r16609983
--- Diff: createmobilespec/createmobilespec.js ---
@@ -210,122 +267,153 @@ if (argv.plugman) {
shelljs.cp("-f", path.join(mobile_spec_git_dir, 'config.xml'), path.join(projectDirName, 'config.xml'));
// Config.json file ---> linked to local libraries
- shelljs.pushd(cli_project_dir);
+ pushd(cli_project_dir);
var localPlatforms = {
- "amazon-fireos" : top_dir + "cordova-amazon-fireos" ,
- "android" : top_dir + "cordova-android" ,
- "ios" : top_dir + "cordova-ios" ,
- "blackberry10" : top_dir + "cordova-blackberry" ,
- "wp8" : top_dir + "cordova-wp8" + path.sep + "wp8",
- "windows8" : top_dir + "cordova-windows",
- "windows" : path.join(top_dir, "cordova-windows", "windows")
+ "amazon-fireos" : [top_dir, "cordova-amazon-fireos"],
+ "android" : [top_dir, "cordova-android"],
+ "ios" : [top_dir, "cordova-ios"],
+ "blackberry10" : [top_dir, "cordova-blackberry"],
+ "wp8" : [top_dir, "cordova-wp8", "wp8"],
+ "windows8" : [top_dir, "cordova-windows"],
+ "windows" : [top_dir, "cordova-windows", "windows"]
};
// Executing platform Add
console.log("Adding platforms...");
- platforms.forEach(function (platform) {
+ [].concat(platforms).forEach(function (platform) {
console.log("Adding Platform: " + platform);
- var platformArg = argv.global ? platform : localPlatforms[platform];
+ var platformArg;
+ if (argv.global) {
+ platformArg = platform;
+ } else {
+ platformArg = path.join.apply(null, localPlatforms[platform]);
+ if (!fs.existsSync(platformArg)) {
+ couldNotFind(localPlatforms[platform][1], platform);
+ platforms = platforms.filter(function (p) { return p != platform; });
+ return;
+ }
+ }
console.log("platformArg: " + cli + " " + platformArg);
shelljs.exec(cli + ' platform add "' + platformArg + '" --verbose');
});
- shelljs.popd();
+ popd();
}
////////////////////// install plugins for each platform
-
-if (argv.plugman) {
- console.log("Adding plugins using plugman...");
- platforms.forEach(function (platform) {
- var projName = getProjName(platform),
- nodeCommand = /^win/.test(process.platform) ? process.argv[0] +" " : "";
- shelljs.pushd(projName);
- // plugin path must be relative and not absolute (sigh)
- shelljs.exec(nodeCommand + path.join(top_dir, "cordova-plugman", "main.js") +
- " install --platform " + platform +
- " --project . --plugin " + path.join("..", "cordova-mobile-spec", "dependencies-plugin") +
- " --searchpath " + top_dir);
- shelljs.popd();
- });
-} else {
- // don't use local git repos for plugins when using --global
- var searchpath = argv.global ? "" : " --searchpath " + top_dir;
- shelljs.pushd(cli_project_dir);
- console.log("Adding plugins using CLI...");
- console.log("Searchpath:", searchpath);
- shelljs.exec(cli + " plugin add " + path.join(mobile_spec_git_dir, "dependencies-plugin") +
- searchpath);
- shelljs.popd();
-}
+function installPlugins() {
+ if (argv.plugman) {
+ console.log("Adding plugins using plugman...");
+ if (!fs.existsSync(path.join(top_dir, "cordova-plugman"))) {
+ couldNotFind('plugman');
+ console.log(" ln -s cordova-lib/cordova-lib cordova-plugman/node_modules");
+ return;
--- End diff --
this needs some `..`'s
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-mobile-spec pull request: CB-7350 Make createmobilespec fr...
Posted by jengee <gi...@git.apache.org>.
Github user jengee commented on a diff in the pull request:
https://github.com/apache/cordova-mobile-spec/pull/103#discussion_r16607454
--- Diff: createmobilespec/createmobilespec.js ---
@@ -210,122 +267,153 @@ if (argv.plugman) {
shelljs.cp("-f", path.join(mobile_spec_git_dir, 'config.xml'), path.join(projectDirName, 'config.xml'));
// Config.json file ---> linked to local libraries
- shelljs.pushd(cli_project_dir);
+ pushd(cli_project_dir);
var localPlatforms = {
- "amazon-fireos" : top_dir + "cordova-amazon-fireos" ,
- "android" : top_dir + "cordova-android" ,
- "ios" : top_dir + "cordova-ios" ,
- "blackberry10" : top_dir + "cordova-blackberry" ,
- "wp8" : top_dir + "cordova-wp8" + path.sep + "wp8",
- "windows8" : top_dir + "cordova-windows",
- "windows" : path.join(top_dir, "cordova-windows", "windows")
+ "amazon-fireos" : [top_dir, "cordova-amazon-fireos"],
+ "android" : [top_dir, "cordova-android"],
+ "ios" : [top_dir, "cordova-ios"],
+ "blackberry10" : [top_dir, "cordova-blackberry"],
+ "wp8" : [top_dir, "cordova-wp8", "wp8"],
+ "windows8" : [top_dir, "cordova-windows"],
+ "windows" : [top_dir, "cordova-windows", "windows"]
};
// Executing platform Add
console.log("Adding platforms...");
- platforms.forEach(function (platform) {
+ [].concat(platforms).forEach(function (platform) {
console.log("Adding Platform: " + platform);
- var platformArg = argv.global ? platform : localPlatforms[platform];
+ var platformArg;
+ if (argv.global) {
+ platformArg = platform;
+ } else {
+ platformArg = path.join.apply(null, localPlatforms[platform]);
+ if (!fs.existsSync(platformArg)) {
+ couldNotFind(localPlatforms[platform][1], platform);
+ platforms = platforms.filter(function (p) { return p != platform; });
+ return;
+ }
+ }
console.log("platformArg: " + cli + " " + platformArg);
shelljs.exec(cli + ' platform add "' + platformArg + '" --verbose');
});
- shelljs.popd();
+ popd();
}
////////////////////// install plugins for each platform
-
-if (argv.plugman) {
- console.log("Adding plugins using plugman...");
- platforms.forEach(function (platform) {
- var projName = getProjName(platform),
- nodeCommand = /^win/.test(process.platform) ? process.argv[0] +" " : "";
- shelljs.pushd(projName);
- // plugin path must be relative and not absolute (sigh)
- shelljs.exec(nodeCommand + path.join(top_dir, "cordova-plugman", "main.js") +
- " install --platform " + platform +
- " --project . --plugin " + path.join("..", "cordova-mobile-spec", "dependencies-plugin") +
- " --searchpath " + top_dir);
- shelljs.popd();
- });
-} else {
- // don't use local git repos for plugins when using --global
- var searchpath = argv.global ? "" : " --searchpath " + top_dir;
- shelljs.pushd(cli_project_dir);
- console.log("Adding plugins using CLI...");
- console.log("Searchpath:", searchpath);
- shelljs.exec(cli + " plugin add " + path.join(mobile_spec_git_dir, "dependencies-plugin") +
- searchpath);
- shelljs.popd();
-}
+function installPlugins() {
+ if (argv.plugman) {
+ console.log("Adding plugins using plugman...");
+ if (!fs.existsSync(path.join(top_dir, "cordova-plugman"))) {
+ couldNotFind('plugman');
+ console.log(" ln -s cordova-lib/cordova-lib cordova-plugman/node_modules");
+ return;
+ }
+ platforms.forEach(function (platform) {
+ var projName = getProjName(platform),
+ nodeCommand = /^win/.test(process.platform) ? process.argv[0] +" " : "";
+ pushd(projName);
+ // plugin path must be relative and not absolute (sigh)
+ shelljs.exec(nodeCommand + path.join(top_dir, "cordova-plugman", "main.js") +
+ " install --platform " + platform +
+ " --project . --plugin " + path.join("..", "cordova-mobile-spec", "dependencies-plugin") +
+ " --searchpath " + top_dir);
+ popd();
+ });
+ } else {
+ // don't use local git repos for plugins when using --global
+ var searchpath = argv.global ? "" : " --searchpath " + top_dir;
+ pushd(cli_project_dir);
+ console.log("Adding plugins using CLI...");
+ console.log("Searchpath:", searchpath);
+ if (!fs.existsSync('cordova-plugin-test-framework')) {
+ couldNotFind('cordova-plugin-test-framework');
+ }
+ shelljs.exec(cli + " plugin add " + path.join(mobile_spec_git_dir, "dependencies-plugin") +
+ searchpath);
+ popd();
+ }
////////////////////// install new-style test plugins
-
-if (argv.plugman) {
- // TODO
-} else {
- shelljs.pushd(cli_project_dir);
- console.log("Adding plugin tests using CLI...");
- shelljs.ls('plugins').forEach(function(plugin) {
- var potential_tests_plugin_xml = path.join('plugins', plugin, 'tests', 'plugin.xml');
- if (fs.existsSync(potential_tests_plugin_xml)) {
- shelljs.exec(cli + " plugin add " + path.dirname(potential_tests_plugin_xml));
- }
- });
- shelljs.popd();
+ if (argv.plugman) {
+ // TODO
+ } else {
+ pushd(cli_project_dir);
+ console.log("Adding plugin tests using CLI...");
+ shelljs.ls('plugins').forEach(function(plugin) {
+ var potential_tests_plugin_xml = path.join('plugins', plugin, 'tests', 'plugin.xml');
+ if (fs.existsSync(potential_tests_plugin_xml)) {
+ shelljs.exec(cli + " plugin add " + path.dirname(potential_tests_plugin_xml));
+ }
+ });
+ popd();
+ }
}
////////////////////// update js files for each platform from cordova-js
-
-if (argv.skipjs) {
- console.log("Skipping the js update.");
-} else if (!argv.global) {
- console.log("Updating js for platforms...");
-
- shelljs.pushd(cordova_js_git_dir);
- var code = shelljs.exec("grunt").code;
- if (code) {
- console.log("Failed to build js.");
- process.exit(1);
+function updateJS() {
+ if (argv.skipjs) {
+ console.log("Skipping the js update.");
+ } else if (!argv.global) {
+ if (!fs.existsSync(cordova_js_git_dir)) {
+ couldNotFind("js", "cordova-js");
+ } else {
+ console.log("Updating js for platforms...");
+
+ pushd(cordova_js_git_dir);
--- End diff --
Something like this is needed:
```js
+ try {
+ require(path.join(cordova_js_git_dir, "node_modules", "grunt"));
+ } catch (e) {
+ console.error("\trun `npm install` from: "+ cordova_js_git_dir);
+ }
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-mobile-spec pull request: CB-7350 Make createmobilespec fr...
Posted by jsoref <gi...@git.apache.org>.
Github user jsoref commented on the pull request:
https://github.com/apache/cordova-mobile-spec/pull/103#issuecomment-52998657
@clelland : ok, I think this is pretty good now...
Android, BlackBerry 10, and iOS are all able to work w/ and w/o `--plugman`
In most of the cases I can imagine, I'm given clear instructions about what to do next if I'm missing something.
@jengee if you could try using this pull request with createmobilespec.js, that'd be great.
I can try to get some other QA to try it too. But, I'd really like to close this pull request tomorrow.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-mobile-spec pull request: CB-7350 Make createmobilespec fr...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-mobile-spec/pull/103
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] cordova-mobile-spec pull request: CB-7350 Make createmobilespec fr...
Posted by jsoref <gi...@git.apache.org>.
Github user jsoref commented on the pull request:
https://github.com/apache/cordova-mobile-spec/pull/103#issuecomment-52998710
-- And by close it, I mean, that I intend to fold all of my commits into 2 or so commits with pretty commit messages and then push that into `apache/master`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---