You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by omefire <gi...@git.apache.org> on 2015/02/09 21:40:54 UTC
[GitHub] cordova-lib pull request: CB-8420 'cordova plugin add' should retr...
GitHub user omefire opened a pull request:
https://github.com/apache/cordova-lib/pull/162
CB-8420 'cordova plugin add' should retrieve version, url or local folder from config.xml if none is specified
CB-8420 'cordova plugin add' should retrieve version, url or local folder from config.xml if none is specified
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MSOpenTech/cordova-lib CB-8420
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-lib/pull/162.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 #162
----
commit 6be67b4c6205f4819887bb34553f75e8d142cdd7
Author: Omar Mefire <om...@microsoft.com>
Date: 2015-02-07T02:03:46Z
CB-8420 'cordova plugin add' should retrieve version, url or local folder from config.xml if none is specified
----
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-8420 'cordova plugin add' should retr...
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/162#issuecomment-73651273
@gorkem Also, this feature does not prevent the other one you mentionned.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-8420 'cordova plugin add' should retr...
Posted by gorkem <gi...@git.apache.org>.
Github user gorkem commented on the pull request:
https://github.com/apache/cordova-lib/pull/162#issuecomment-73638713
-1 since we agreed to have auto restore for plugins I do not think this feature will be used at all.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
Re: [GitHub] cordova-lib pull request: CB-8420 'cordova plugin add'
should retr...
Posted by Andrew Grieve <ag...@chromium.org>.
ah, I missed that lstatSync throws an exception for non-string args. I
suppose that's fine. Will merge!
On Wed, Feb 11, 2015 at 2:54 PM, omefire <gi...@git.apache.org> wrote:
> Github user omefire commented on a diff in the pull request:
>
> https://github.com/apache/cordova-lib/pull/162#discussion_r24527527
>
> --- Diff: cordova-lib/src/cordova/plugin.js ---
> @@ -114,6 +116,22 @@ module.exports = function plugin(command,
> targets, opts) {
> target = target.substring(0,
> target.length - 1);
> }
>
> + var parts = target.split('@');
> + var id = parts[0];
> + var version = parts[1];
> +
> + // If no version is specified, retrieve the
> version from config.xml
> + if(!version && !cordova_util.isUrl(id) &&
> !cordova_util.isDirectory(id)){
> + events.emit('verbose', 'no version
> specified, retrieving version from config.xml');
> + var ver = getVersionFromConfigFile(id,
> cfg);
> +
> + if( cordova_util.isUrl(ver) ||
> cordova_util.isDirectory(ver) ){
> --- End diff --
>
> @agrieve, isDirectory does handle the null/undefined cases. I just
> tested it. Am I missing something ?
>
>
> ---
> 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.
> ---
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
> For additional commands, e-mail: dev-help@cordova.apache.org
>
>
[GitHub] cordova-lib pull request: CB-8420 'cordova plugin add' should retr...
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/162#discussion_r24527527
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -114,6 +116,22 @@ module.exports = function plugin(command, targets, opts) {
target = target.substring(0, target.length - 1);
}
+ var parts = target.split('@');
+ var id = parts[0];
+ var version = parts[1];
+
+ // If no version is specified, retrieve the version from config.xml
+ if(!version && !cordova_util.isUrl(id) && !cordova_util.isDirectory(id)){
+ events.emit('verbose', 'no version specified, retrieving version from config.xml');
+ var ver = getVersionFromConfigFile(id, cfg);
+
+ if( cordova_util.isUrl(ver) || cordova_util.isDirectory(ver) ){
--- End diff --
@agrieve, isDirectory does handle the null/undefined cases. I just tested it. Am I missing something ?
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-8420 'cordova plugin add' should retr...
Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on the pull request:
https://github.com/apache/cordova-lib/pull/162#issuecomment-73895354
lgtm save one nit. I think this behaviour is fine. Normal case will be to use add --save anyways.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-8420 'cordova plugin add' should retr...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-lib/pull/162
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-8420 'cordova plugin add' should retr...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the pull request:
https://github.com/apache/cordova-lib/pull/162#issuecomment-73652050
I'm not aware of overall design of save/restore features, but code 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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-8420 'cordova plugin add' should retr...
Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/162#discussion_r24500705
--- Diff: cordova-lib/src/cordova/plugin.js ---
@@ -114,6 +116,22 @@ module.exports = function plugin(command, targets, opts) {
target = target.substring(0, target.length - 1);
}
+ var parts = target.split('@');
+ var id = parts[0];
+ var version = parts[1];
+
+ // If no version is specified, retrieve the version from config.xml
+ if(!version && !cordova_util.isUrl(id) && !cordova_util.isDirectory(id)){
+ events.emit('verbose', 'no version specified, retrieving version from config.xml');
+ var ver = getVersionFromConfigFile(id, cfg);
+
+ if( cordova_util.isUrl(ver) || cordova_util.isDirectory(ver) ){
--- End diff --
isDirectory don't check for null/undefined. Should either add that to isDirectory, or add check here.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-8420 'cordova plugin add' should retr...
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/162#issuecomment-73615544
@vladimir-kotikov, @agrieve and @gorkem , please review.
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-8420 'cordova plugin add' should retr...
Posted by omefire <gi...@git.apache.org>.
Github user omefire commented on the pull request:
https://github.com/apache/cordova-lib/pull/162#issuecomment-73641701
Well, if you review the document we all agreed on, it mentions this. Plus, this feature is already in for platforms as well. I'm trying to mirror it : https://docs.google.com/document/d/1qKjhzSf48ybGg2GFZPtjXP8dkF4Z5Jnc7FU41V3-jFc/edit#heading=h.tsqn5em3043l
---
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.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org