[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[stevengill]

Github user stevengill closed the pull request at:

https://github.com/apache/cordova-lib/pull/407


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[stevengill]

Github user stevengill commented on the pull request:

https://github.com/apache/cordova-lib/pull/407#issuecomment-21866
  
Merged 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[vladimir-kotikov]

Github user vladimir-kotikov commented on the pull request:

https://github.com/apache/cordova-lib/pull/407#issuecomment-217091665
  
LGTM


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[stevengill]

Github user stevengill commented on the pull request:

https://github.com/apache/cordova-lib/pull/407#issuecomment-217087853
  
Made the changes. LMK. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[vladimir-kotikov]

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/407#discussion_r62104877
  
--- Diff: cordova-fetch/spec/package.json ---
@@ -0,0 +1,11 @@
+{
--- End diff --

Hmm. Maybe just name it differently. My concern is that presence of another 
`package.json` might be a bit confusing + break some tools in some scenarios


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[stevengill]

Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/407#discussion_r62101897
  
--- Diff: cordova-fetch/spec/package.json ---
@@ -0,0 +1,11 @@
+{
--- End diff --

I just copy it into temporary tests folder so I can test - - save. Any 
alternative suggestion? Should I store it somewhere else? 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[stevengill]

Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/407#discussion_r62100822
  
--- Diff: cordova-fetch/index.js ---
@@ -0,0 +1,234 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ 'License'); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ 'AS IS' BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied.  See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */
+
+var Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/* 
+ * A function that npm installs a module from npm or a git url
+ *
+ * @param {String} target   the packageID or git url
+ * @param {String} dest destination of where to install the module
+ * @param {Object} opts [opts={save:true}] options to pass to fetch 
module
+ *
+ * @return {String||Promise}Returns string of the absolute path to the 
installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+var fetchArgs = ['install'];
+opts = opts || {};
+var tree1;
+
+//check if npm is installed
+isNpmInstalled();
--- End diff --

Your right. I originally has it connected to the promise change but removed 
it during a refactor. I'll fix this! 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[stevengill]

Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/407#discussion_r62100600
  
--- Diff: cordova-fetch/index.js ---
@@ -0,0 +1,234 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ 'License'); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ 'AS IS' BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied.  See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */
+
+var Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/* 
+ * A function that npm installs a module from npm or a git url
+ *
+ * @param {String} target   the packageID or git url
+ * @param {String} dest destination of where to install the module
+ * @param {Object} opts [opts={save:true}] options to pass to fetch 
module
+ *
+ * @return {String||Promise}Returns string of the absolute path to the 
installed module.
--- End diff --

Thanks! I'll fix that


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[vladimir-kotikov]

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/407#discussion_r62000656
  
--- Diff: cordova-fetch/index.js ---
@@ -0,0 +1,234 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ 'License'); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ 'AS IS' BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied.  See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */
+
+var Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/* 
+ * A function that npm installs a module from npm or a git url
+ *
+ * @param {String} target   the packageID or git url
+ * @param {String} dest destination of where to install the module
+ * @param {Object} opts [opts={save:true}] options to pass to fetch 
module
+ *
+ * @return {String||Promise}Returns string of the absolute path to the 
installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+var fetchArgs = ['install'];
+opts = opts || {};
+var tree1;
+
+//check if npm is installed
+isNpmInstalled();
+
+if(dest && target) {
+//add target to fetchArgs Array
+fetchArgs.push(target);
+
+//append node_modules to dest if it doesn't come included
+if (path.basename(dest) !== 'node_modules') {
+dest = path.resolve(path.join(dest, 'node_modules'));
+}
+
+//create dest if it doesn't exist
+if(!fs.existsSync(dest)) {
+shell.mkdir('-p', dest); 
+} 
+
+} else return Q.reject(new CordovaError('Need to supply a target and 
destination'));
+
+//set the directory where npm install will be run
+opts.cwd = dest;
+
+//if user added --save flag, pass it to npm install command
+if(opts.save) {
+events.emit('verbose', 'saving');
+fetchArgs.push('--save'); 
+} 
+
+
+//Grab json object of installed modules before npm install
+return depls(dest)
+.then(function(depTree) {
+tree1 = depTree;
+
+//install new module
+return superspawn.spawn('npm', fetchArgs, opts);
+})
+.then(function(output) {
+//Grab object of installed modules after npm install
+return depls(dest);
+})
+.then(function(depTree2) {
+var tree2 = depTree2;
+
+//getJsonDiff will fail if the module already exists in 
node_modules.
+//Need to use trimID in that case. 
+//This could happen on a platform update.
+var id = getJsonDiff(tree1, tree2) || trimID(target); 
+
+return getPath(id, dest);
+}) 
+.fail(function(err){
+return Q.reject(new CordovaError(err));
+});
+};
+
+
+/*
+ * Takes two JSON objects and returns the key of the new property as a 
string.
+ * If a module already exists in node_modules, the diff will be blank. 
+ * cordova-fetch will use trimID in that case.
+ *
+ * @param {Object} obj1 json object representing installed modules 
before latest npm install
+ * @param {Object} obj2 json object representing installed modules 
after latest npm install
+ *
+ * @return {String} String containing the key value of the 
difference between the two objects
+ *
+ */
+function getJsonDiff(obj1, obj2) {
+var result = '';
+
+//regex to filter out peer dependency warnings from result
+var re = /UNMET PEER DEPENDENCY/;
+
+for (var key in obj2) {
+//if it isn't a unmet peer dependency, continue
+if (key.search(re) === -1) {
+if(obj2[key] != obj1[key]) result = key;
+}
+}
+return result;
+}
+
+/*
+ * Takes the 

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[vladimir-kotikov]

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/407#discussion_r61999817
  
--- Diff: cordova-fetch/spec/package.json ---
@@ -0,0 +1,11 @@
+{
--- End diff --

Do we need a separate `package.json` for specs?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[vladimir-kotikov]

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/407#discussion_r61998473
  
--- Diff: cordova-fetch/index.js ---
@@ -0,0 +1,234 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ 'License'); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ 'AS IS' BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied.  See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */
+
+var Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/* 
+ * A function that npm installs a module from npm or a git url
+ *
+ * @param {String} target   the packageID or git url
+ * @param {String} dest destination of where to install the module
+ * @param {Object} opts [opts={save:true}] options to pass to fetch 
module
+ *
+ * @return {String||Promise}Returns string of the absolute path to the 
installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+var fetchArgs = ['install'];
+opts = opts || {};
+var tree1;
+
+//check if npm is installed
+isNpmInstalled();
+
+if(dest && target) {
+//add target to fetchArgs Array
+fetchArgs.push(target);
+
+//append node_modules to dest if it doesn't come included
+if (path.basename(dest) !== 'node_modules') {
+dest = path.resolve(path.join(dest, 'node_modules'));
+}
+
+//create dest if it doesn't exist
+if(!fs.existsSync(dest)) {
+shell.mkdir('-p', dest); 
+} 
+
+} else return Q.reject(new CordovaError('Need to supply a target and 
destination'));
+
+//set the directory where npm install will be run
+opts.cwd = dest;
+
+//if user added --save flag, pass it to npm install command
+if(opts.save) {
+events.emit('verbose', 'saving');
+fetchArgs.push('--save'); 
+} 
+
+
+//Grab json object of installed modules before npm install
+return depls(dest)
+.then(function(depTree) {
+tree1 = depTree;
+
+//install new module
+return superspawn.spawn('npm', fetchArgs, opts);
+})
+.then(function(output) {
+//Grab object of installed modules after npm install
+return depls(dest);
+})
+.then(function(depTree2) {
+var tree2 = depTree2;
+
+//getJsonDiff will fail if the module already exists in 
node_modules.
+//Need to use trimID in that case. 
+//This could happen on a platform update.
+var id = getJsonDiff(tree1, tree2) || trimID(target); 
+
+return getPath(id, dest);
+}) 
+.fail(function(err){
+return Q.reject(new CordovaError(err));
+});
+};
+
+
+/*
+ * Takes two JSON objects and returns the key of the new property as a 
string.
+ * If a module already exists in node_modules, the diff will be blank. 
+ * cordova-fetch will use trimID in that case.
+ *
+ * @param {Object} obj1 json object representing installed modules 
before latest npm install
+ * @param {Object} obj2 json object representing installed modules 
after latest npm install
+ *
+ * @return {String} String containing the key value of the 
difference between the two objects
+ *
+ */
+function getJsonDiff(obj1, obj2) {
+var result = '';
+
+//regex to filter out peer dependency warnings from result
+var re = /UNMET PEER DEPENDENCY/;
+
+for (var key in obj2) {
+//if it isn't a unmet peer dependency, continue
+if (key.search(re) === -1) {
+if(obj2[key] != obj1[key]) result = key;
+}
+}
+return result;
+}
+
+/*
+ * Takes the 

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[vladimir-kotikov]

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/407#discussion_r61998292
  
--- Diff: cordova-fetch/index.js ---
@@ -0,0 +1,234 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ 'License'); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ 'AS IS' BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied.  See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */
+
+var Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/* 
+ * A function that npm installs a module from npm or a git url
+ *
+ * @param {String} target   the packageID or git url
+ * @param {String} dest destination of where to install the module
+ * @param {Object} opts [opts={save:true}] options to pass to fetch 
module
+ *
+ * @return {String||Promise}Returns string of the absolute path to the 
installed module.
--- End diff --

Nit: the JSDoc notation for multiple types is a bit different - single `|` 
instead `||` (please see http://usejsdoc.org/tags-type.html).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[stevengill]

Github user stevengill commented on the pull request:

https://github.com/apache/cordova-lib/pull/407#issuecomment-216691951
  
Hey Everyone,

I think the first implementation of this is ready to hit master behind the 
--fetch flag. It includes platform and plugin fetching + removing. 

Take a look @nikhilkh @purplecabbage @vladimir-kotikov 

Needs: apache/cordova-cli#239 and apache/cordova-plugman#87

I will add template fetching next.

I did run into some issues with our cordova tests. The tests finish before 
the promise resolves. Probably a issue with how we are handling promises in 
cordova, but could also be related to us using jasmine 1.3 instead of jasmine 
2.x. I'll continue to investigate but not a blocker. My manually testing shows 
that the uninstall command works fine with cordova.

cordova-fetch tests are written using jasmine 2.0. No issues there. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[codecov-io]

Github user codecov-io commented on the pull request:

https://github.com/apache/cordova-lib/pull/407#issuecomment-215918679
  
## [Current coverage][cc-pull] is **13.19%**
> Merging [#407][cc-pull] into [master][cc-base-branch] will increase 
coverage by **-86.81%**

```diff
@@ master   #407   diff @@
==
  Files69 34 -35   
  Lines  9878   3261   -6617   
  Methods 841525-316   
  Messages  0  0   
  Branches   1271636-635   
==
- Hits   9878430   -9448   
- Misses0   2831   +2831   
  Partials  0  0   
```

1. 3 files (not in diff) in `cordova-lib/src/util` were modified. 
[more](https://codecov.io/gh/apache/cordova-lib/commit/b61258fd4d50e6f1b71b55a6200a4e73b7d1857f/changes?src=pr#636F72646F76612D6C69622F7372632F7574696C)
 
  - Misses `+46` 
  - Hits `-86`
1. 4 files (not in diff) in `...dova-lib/src/plugman` were modified. 
[more](https://codecov.io/gh/apache/cordova-lib/commit/b61258fd4d50e6f1b71b55a6200a4e73b7d1857f/changes?src=pr#636F72646F76612D6C69622F7372632F706C75676D616E)
 
  - Misses `+219` 
  - Hits `-437`
1. 2 files (not in diff) in `...va-lib/src/platforms` were modified. 
[more](https://codecov.io/gh/apache/cordova-lib/commit/b61258fd4d50e6f1b71b55a6200a4e73b7d1857f/changes?src=pr#636F72646F76612D6C69622F7372632F706C6174666F726D73)
 
  - Misses `+234` 
  - Hits `-444`
1. 3 files (not in diff) in `...ordova-lib/src/hooks` were modified. 
[more](https://codecov.io/gh/apache/cordova-lib/commit/b61258fd4d50e6f1b71b55a6200a4e73b7d1857f/changes?src=pr#636F72646F76612D6C69622F7372632F686F6F6B73)
 
  - Misses `+159` 
  - Hits `-315`
1. 2 files (not in diff) in `...etadata/parserhelper` were modified. 
[more](https://codecov.io/gh/apache/cordova-lib/commit/b61258fd4d50e6f1b71b55a6200a4e73b7d1857f/changes?src=pr#636F72646F76612D6C69622F7372632F636F72646F76612F6D657461646174612F70617273657268656C706572)
 
  - Misses `+24` 
  - Hits `-84`
1. 9 files (not in diff) in `...src/cordova/metadata` were modified. 
[more](https://codecov.io/gh/apache/cordova-lib/commit/b61258fd4d50e6f1b71b55a6200a4e73b7d1857f/changes?src=pr#636F72646F76612D6C69622F7372632F636F72646F76612F6D65746164617461)
 
  - Misses `+859` 
  - Hits `-1578`
1. 7 files (not in diff) in `...dova-lib/src/cordova` were modified. 
[more](https://codecov.io/gh/apache/cordova-lib/commit/b61258fd4d50e6f1b71b55a6200a4e73b7d1857f/changes?src=pr#636F72646F76612D6C69622F7372632F636F72646F7661)
 
  - Misses `+440` 
  - Hits `-855`
1. 1 files (not in diff) in `cordova-lib/src` were modified. 
[more](https://codecov.io/gh/apache/cordova-lib/commit/b61258fd4d50e6f1b71b55a6200a4e73b7d1857f/changes?src=pr#636F72646F76612D6C69622F737263)
 
  - Misses `+22` 
  - Hits `-42`
1. 2 files in `...dova-lib/src/cordova` were modified. 
[more](https://codecov.io/gh/apache/cordova-lib/commit/b61258fd4d50e6f1b71b55a6200a4e73b7d1857f/changes?src=pr#636F72646F76612D6C69622F7372632F636F72646F7661)
 
  - Misses `+716` 
  - Hits `-1327`
1. 1 files in `cordova-lib/src` were modified. 
[more](https://codecov.io/gh/apache/cordova-lib/commit/b61258fd4d50e6f1b71b55a6200a4e73b7d1857f/changes?src=pr#636F72646F76612D6C69622F737263)
 
  - Misses `+73` 
  - Hits `-136`


> Powered by [Codecov](https://codecov.io?src=pr). Last updated by 
[5867657...b61258f][cc-compare]
[cc-base-branch]: 
https://codecov.io/gh/apache/cordova-lib/branch/master?src=pr
[cc-compare]: 
https://codecov.io/gh/apache/cordova-lib/compare/5867657d989b136702fe2f14172661ca489573ed...b61258fd4d50e6f1b71b55a6200a4e73b7d1857f
[cc-pull]: https://codecov.io/gh/apache/cordova-lib/pull/407?src=pr


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[stevengill]

Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/407#discussion_r60995427
  
--- Diff: cordova-fetch/index.js ---
@@ -0,0 +1,226 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ 'License'); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ 'AS IS' BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied.  See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */
+
+var Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/* 
+ * A module that npm installs a module from npm or a git url
+ *
+ * @param {String} target   the packageID or git url
+ * @param {String} dest destination of where to install the module
+ * @param {Object} opts [opts={save:true}] options to pass to fetch 
module
+ *
+ * @return {String||Promise}Returns string of the absolute path to the 
installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+var fetchArgs = ['install'];
+opts = opts || {};
+var tree1;
+
+if(dest && target) {
+//add target to fetchArgs Array
+fetchArgs.push(target);
+
+//append node_modules to dest if it doesn't come included
+if (path.basename(dest) !== 'node_modules') {
+dest = path.resolve(path.join(dest, 'node_modules'));
+}
+
+//create dest if it doesn't exist
+if(!fs.existsSync(dest)) {
+shell.mkdir('-p', dest); 
+} 
+
+} else return Q.reject(new CordovaError('Need to supply a target and 
destination'));
+
+//set the directory where npm install will be run
+opts.cwd = dest;
+
+//if user added --save flag, pass it to npm install command
+if(opts.save) {
+events.emit('verbose', 'saving');
+fetchArgs.push('--save'); 
+} 
+
+//check if npm is installed
+isNpmInstalled();
+
+//Grab json object of installed modules before npm install
+return depls(dest)
+.then(function(depTree) {
+tree1 = depTree;
+
+//install new module
+return superspawn.spawn('npm', fetchArgs, opts);
--- End diff --

I don't think it is necessary. It is running `npm install` for modules from 
registry or git url. `devDependencies` shouldn't be getting installed in this 
scenario regardless of `--production`


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[stevengill]

Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/407#discussion_r60995226
  
--- Diff: cordova-fetch/index.js ---
@@ -0,0 +1,226 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ 'License'); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ 'AS IS' BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied.  See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */
+
+var Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/* 
+ * A module that npm installs a module from npm or a git url
+ *
+ * @param {String} target   the packageID or git url
+ * @param {String} dest destination of where to install the module
+ * @param {Object} opts [opts={save:true}] options to pass to fetch 
module
+ *
+ * @return {String||Promise}Returns string of the absolute path to the 
installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+var fetchArgs = ['install'];
+opts = opts || {};
+var tree1;
+
+if(dest && target) {
+//add target to fetchArgs Array
+fetchArgs.push(target);
+
+//append node_modules to dest if it doesn't come included
+if (path.basename(dest) !== 'node_modules') {
+dest = path.resolve(path.join(dest, 'node_modules'));
+}
+
+//create dest if it doesn't exist
+if(!fs.existsSync(dest)) {
+shell.mkdir('-p', dest); 
+} 
+
+} else return Q.reject(new CordovaError('Need to supply a target and 
destination'));
+
+//set the directory where npm install will be run
+opts.cwd = dest;
+
+//if user added --save flag, pass it to npm install command
+if(opts.save) {
+events.emit('verbose', 'saving');
+fetchArgs.push('--save'); 
+} 
+
+//check if npm is installed
+isNpmInstalled();
--- End diff --

True. I'll move it up


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[vladimir-kotikov]

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/407#discussion_r60899092
  
--- Diff: cordova-fetch/index.js ---
@@ -0,0 +1,226 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ 'License'); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ 'AS IS' BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied.  See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */
+
+var Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/* 
+ * A module that npm installs a module from npm or a git url
+ *
+ * @param {String} target   the packageID or git url
+ * @param {String} dest destination of where to install the module
+ * @param {Object} opts [opts={save:true}] options to pass to fetch 
module
+ *
+ * @return {String||Promise}Returns string of the absolute path to the 
installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+var fetchArgs = ['install'];
+opts = opts || {};
+var tree1;
+
+if(dest && target) {
+//add target to fetchArgs Array
+fetchArgs.push(target);
+
+//append node_modules to dest if it doesn't come included
+if (path.basename(dest) !== 'node_modules') {
+dest = path.resolve(path.join(dest, 'node_modules'));
+}
+
+//create dest if it doesn't exist
+if(!fs.existsSync(dest)) {
+shell.mkdir('-p', dest); 
+} 
+
+} else return Q.reject(new CordovaError('Need to supply a target and 
destination'));
+
+//set the directory where npm install will be run
+opts.cwd = dest;
+
+//if user added --save flag, pass it to npm install command
+if(opts.save) {
+events.emit('verbose', 'saving');
+fetchArgs.push('--save'); 
+} 
+
+//check if npm is installed
+isNpmInstalled();
+
+//Grab json object of installed modules before npm install
+return depls(dest)
+.then(function(depTree) {
+tree1 = depTree;
+
+//install new module
+return superspawn.spawn('npm', fetchArgs, opts);
--- End diff --

Perhaps it is worth to add `--production` flag to avoid installing 
`devDependencies`


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[vladimir-kotikov]

Github user vladimir-kotikov commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/407#discussion_r60898584
  
--- Diff: cordova-fetch/index.js ---
@@ -0,0 +1,226 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ 'License'); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ 'AS IS' BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied.  See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */
+
+var Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/* 
+ * A module that npm installs a module from npm or a git url
+ *
+ * @param {String} target   the packageID or git url
+ * @param {String} dest destination of where to install the module
+ * @param {Object} opts [opts={save:true}] options to pass to fetch 
module
+ *
+ * @return {String||Promise}Returns string of the absolute path to the 
installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+var fetchArgs = ['install'];
+opts = opts || {};
+var tree1;
+
+if(dest && target) {
+//add target to fetchArgs Array
+fetchArgs.push(target);
+
+//append node_modules to dest if it doesn't come included
+if (path.basename(dest) !== 'node_modules') {
+dest = path.resolve(path.join(dest, 'node_modules'));
+}
+
+//create dest if it doesn't exist
+if(!fs.existsSync(dest)) {
+shell.mkdir('-p', dest); 
+} 
+
+} else return Q.reject(new CordovaError('Need to supply a target and 
destination'));
+
+//set the directory where npm install will be run
+opts.cwd = dest;
+
+//if user added --save flag, pass it to npm install command
+if(opts.save) {
+events.emit('verbose', 'saving');
+fetchArgs.push('--save'); 
+} 
+
+//check if npm is installed
+isNpmInstalled();
--- End diff --

Probably it makes sense to check this at the very beginning of the method


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[stevengill]

Github user stevengill commented on the pull request:

https://github.com/apache/cordova-lib/pull/407#issuecomment-199661470
  
Plugman PR adding --fetch flag support 
https://github.com/apache/cordova-plugman/pull/87


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[nikhilkh]

Github user nikhilkh commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/407#discussion_r55567900
  
--- Diff: cordova-fetch/index.js ---
@@ -0,0 +1,176 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ 'License'); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ 'AS IS' BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied.  See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */
+
+var Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/* 
+ * A module that npm installs a module from npm or a git url
+ *
+ * @param {String} target   the packageID or git url
+ * @param {String} dest destination of where to install the module
+ * @param {Object} opts [opts={save:true}] options to pass to fetch 
module
+ *
+ * @return {String||Promise}Returns string of the absolute path to the 
installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+var fetchArgs = ['install'];
+opts = opts || {};
+var tree1;
+
+if(!shell.which('npm')) {
--- End diff --


https://github.com/apache/cordova-android/blob/master/bin/lib/check_reqs.js#L97.
 We end up modifying process.env["PATH"] with probable directories for 
JAVA_HOME/ANDROID_HOME  before invoking superspawn. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[stevengill]

Github user stevengill commented on the pull request:

https://github.com/apache/cordova-lib/pull/407#issuecomment-194441672
  
To-do: add cordova-lib tests for this




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

Re: [GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[manish varma]

cool

On Wed, Mar 9, 2016 at 12:53 PM, stevengill  wrote:

> Github user stevengill commented on a diff in the pull request:
>
> https://github.com/apache/cordova-lib/pull/407#discussion_r55480177
>
> --- Diff: cordova-fetch/index.js ---
> @@ -0,0 +1,176 @@
> +/**
> + Licensed to the Apache Software Foundation (ASF) under one
> + or more contributor license agreements.  See the NOTICE file
> + distributed with this work for additional information
> + regarding copyright ownership.  The ASF licenses this file
> + to you under the Apache License, Version 2.0 (the
> + 'License'); you may not use this file except in compliance
> + with the License.  You may obtain a copy of the License at
> +
> + http://www.apache.org/licenses/LICENSE-2.0
> +
> + Unless required by applicable law or agreed to in writing,
> + software distributed under the License is distributed on an
> + 'AS IS' BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
> + KIND, either express or implied.  See the License for the
> + specific language governing permissions and limitations
> + under the License.
> + */
> +
> +var Q = require('q');
> +var shell = require('shelljs');
> +var superspawn = require('cordova-common').superspawn;
> +var events = require('cordova-common').events;
> +var depls = require('dependency-ls');
> +var path = require('path');
> +var fs = require('fs');
> +var CordovaError = require('cordova-common').CordovaError;
> +var isUrl = require('is-url');
> +
> +/*
> + * A module that npm installs a module from npm or a git url
> + *
> + * @param {String} target   the packageID or git url
> + * @param {String} dest destination of where to install the module
> + * @param {Object} opts [opts={save:true}] options to pass to
> fetch module
> + *
> + * @return {String||Promise}Returns string of the absolute path
> to the installed module.
> + *
> + */
> +module.exports = function(target, dest, opts) {
> +var fetchArgs = ['install'];
> +opts = opts || {};
> +var tree1;
> +
> +if(!shell.which('npm')) {
> +return Q.reject(new CordovaError('"npm" command line tool is
> not installed: make sure it is accessible on your PATH.'));
> +}
> +
> +if(dest && target) {
> +//add target to fetchArgs Array
> +fetchArgs.push(target);
> +
> +//append node_modules to dest if it doesn't come included
> +if (path.basename(dest) !== 'node_modules') {
> +dest = path.resolve(path.join(dest, 'node_modules'));
> +}
> +
> +//create dest if it doesn't exist
> +if(!fs.existsSync(dest)) {
> +shell.mkdir('-p', dest);
> +}
> +
> +} else return Q.reject(new CordovaError('Need to supply a target
> and destination'));
> +
> +//set the directory where npm install will be run
> +opts.cwd = dest;
> --- End diff --
>
> Yup! Pass everything to superspawn that fetch was passed. Add the cwd
> to opts though.
>
>
> ---
> 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 infrastruct...@apache.org or file a JIRA ticket
> with INFRA.
> ---
>
> -
> To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
> For additional commands, e-mail: dev-h...@cordova.apache.org
>
>


-- 
Regards
Manish Varma D
+91-9538612399
LinkedIn: //in.linkedin.com/pub/manish-varma-d/86/66a/ab
Think Green: Please consider the environment before printing this e-mail.Privy
Notice: This is a confidential email. If you are not the expected
recipient, we alert you that this email has reached you by error and
disclosing of the contents to any other source is strictly prohibited. If
you have received this email by error, you are requested to acknowledge and
delete the message immediately.

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[stevengill]

Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/407#discussion_r55480177
  
--- Diff: cordova-fetch/index.js ---
@@ -0,0 +1,176 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ 'License'); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ 'AS IS' BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied.  See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */
+
+var Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/* 
+ * A module that npm installs a module from npm or a git url
+ *
+ * @param {String} target   the packageID or git url
+ * @param {String} dest destination of where to install the module
+ * @param {Object} opts [opts={save:true}] options to pass to fetch 
module
+ *
+ * @return {String||Promise}Returns string of the absolute path to the 
installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+var fetchArgs = ['install'];
+opts = opts || {};
+var tree1;
+
+if(!shell.which('npm')) {
+return Q.reject(new CordovaError('"npm" command line tool is not 
installed: make sure it is accessible on your PATH.'));
+}
+
+if(dest && target) {
+//add target to fetchArgs Array
+fetchArgs.push(target);
+
+//append node_modules to dest if it doesn't come included
+if (path.basename(dest) !== 'node_modules') {
+dest = path.resolve(path.join(dest, 'node_modules'));
+}
+
+//create dest if it doesn't exist
+if(!fs.existsSync(dest)) {
+shell.mkdir('-p', dest); 
+} 
+
+} else return Q.reject(new CordovaError('Need to supply a target and 
destination'));
+
+//set the directory where npm install will be run
+opts.cwd = dest;
--- End diff --

Yup! Pass everything to superspawn that fetch was passed. Add the cwd to 
opts though. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[stevengill]

Github user stevengill commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/407#discussion_r55480086
  
--- Diff: cordova-fetch/index.js ---
@@ -0,0 +1,176 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ 'License'); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ 'AS IS' BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied.  See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */
+
+var Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/* 
+ * A module that npm installs a module from npm or a git url
+ *
+ * @param {String} target   the packageID or git url
+ * @param {String} dest destination of where to install the module
+ * @param {Object} opts [opts={save:true}] options to pass to fetch 
module
+ *
+ * @return {String||Promise}Returns string of the absolute path to the 
installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+var fetchArgs = ['install'];
+opts = opts || {};
+var tree1;
+
+if(!shell.which('npm')) {
--- End diff --

Sounds like a good idea! Do you have any examples of doing this with 
Java_home I could use as a basis for this? I pretty much mickimed how our 
current fetch code looks for git. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[nikhilkh]

Github user nikhilkh commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/407#discussion_r55473246
  
--- Diff: cordova-fetch/index.js ---
@@ -0,0 +1,176 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ 'License'); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ 'AS IS' BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied.  See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */
+
+var Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/* 
+ * A module that npm installs a module from npm or a git url
+ *
+ * @param {String} target   the packageID or git url
+ * @param {String} dest destination of where to install the module
+ * @param {Object} opts [opts={save:true}] options to pass to fetch 
module
+ *
+ * @return {String||Promise}Returns string of the absolute path to the 
installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+var fetchArgs = ['install'];
+opts = opts || {};
+var tree1;
+
+if(!shell.which('npm')) {
+return Q.reject(new CordovaError('"npm" command line tool is not 
installed: make sure it is accessible on your PATH.'));
+}
+
+if(dest && target) {
+//add target to fetchArgs Array
+fetchArgs.push(target);
+
+//append node_modules to dest if it doesn't come included
+if (path.basename(dest) !== 'node_modules') {
+dest = path.resolve(path.join(dest, 'node_modules'));
+}
+
+//create dest if it doesn't exist
+if(!fs.existsSync(dest)) {
+shell.mkdir('-p', dest); 
+} 
+
+} else return Q.reject(new CordovaError('Need to supply a target and 
destination'));
+
+//set the directory where npm install will be run
+opts.cwd = dest;
--- End diff --

Are the `opts` for superspawn the same as the `opts` passed to this 
function?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org

[GitHub] cordova-lib pull request: CB-9858 Cordova Fetch Work

[nikhilkh]

Github user nikhilkh commented on a diff in the pull request:

https://github.com/apache/cordova-lib/pull/407#discussion_r55472886
  
--- Diff: cordova-fetch/index.js ---
@@ -0,0 +1,176 @@
+/**
+ Licensed to the Apache Software Foundation (ASF) under one
+ or more contributor license agreements.  See the NOTICE file
+ distributed with this work for additional information
+ regarding copyright ownership.  The ASF licenses this file
+ to you under the Apache License, Version 2.0 (the
+ 'License'); you may not use this file except in compliance
+ with the License.  You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing,
+ software distributed under the License is distributed on an
+ 'AS IS' BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ KIND, either express or implied.  See the License for the
+ specific language governing permissions and limitations
+ under the License.
+ */
+
+var Q = require('q');
+var shell = require('shelljs');
+var superspawn = require('cordova-common').superspawn;
+var events = require('cordova-common').events;
+var depls = require('dependency-ls');
+var path = require('path');
+var fs = require('fs');
+var CordovaError = require('cordova-common').CordovaError;
+var isUrl = require('is-url');
+
+/* 
+ * A module that npm installs a module from npm or a git url
+ *
+ * @param {String} target   the packageID or git url
+ * @param {String} dest destination of where to install the module
+ * @param {Object} opts [opts={save:true}] options to pass to fetch 
module
+ *
+ * @return {String||Promise}Returns string of the absolute path to the 
installed module.
+ *
+ */
+module.exports = function(target, dest, opts) {
+var fetchArgs = ['install'];
+opts = opts || {};
+var tree1;
+
+if(!shell.which('npm')) {
--- End diff --

Judging from previous experience with our tools not being able to find 
ANDROID_HOME/JAVA_HOME - this might become an error message that will affect 
new people. We should consider doing a best effort to find the npm installed on 
the system if `which` can't find it. There are some well known places on 
Windows/Mac where this gets installed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: dev-unsubscr...@cordova.apache.org
For additional commands, e-mail: dev-h...@cordova.apache.org