[GitHub] cordova-lib pull request: CB-8225: Add Unit Tests for platform.js/...

2014-12-30 Thread omefire
GitHub user omefire opened a pull request: https://github.com/apache/cordova-lib/pull/137 CB-8225: Add Unit Tests for platform.js/add function CB-8225: Add Unit Tests for platform.js/add function - helps prevent regressions to this function - helps familiarize with cordova

[GitHub] cordova-lib pull request: CB-8225: Add Unit Tests for platform.js/...

2014-12-30 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/137#issuecomment-68418329 Just realized that this commit changes one of the behaviors I was testing for: https://github.com/apache/cordova-lib/commit

[GitHub] cordova-lib pull request: CB-8225: Add Unit Tests for platform.js/...

2014-12-30 Thread omefire
Github user omefire closed the pull request at: https://github.com/apache/cordova-lib/pull/137 --- 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

[GitHub] cordova-lib pull request: CB-8225: Add Unit Tests for platform.js/...

2014-12-30 Thread omefire
GitHub user omefire opened a pull request: https://github.com/apache/cordova-lib/pull/138 CB-8225: Add Unit Tests for platform.js/add function CB-8225: Add Unit Tests for platform.js/add function : - Help prevent regressions to add function - Get feet wet with cordova

[GitHub] cordova-lib pull request: CB-8226 'cordova platform add' : Look up...

2015-01-02 Thread omefire
GitHub user omefire opened a pull request: https://github.com/apache/cordova-lib/pull/139 CB-8226 'cordova platform add' : Look up version in config.xml if no version specified CB-8226 'cordova platform add' : Look up version in config.xml if no version

[GitHub] cordova-lib pull request: CB-8226 'cordova platform add' : Look up...

2015-01-02 Thread omefire
Github user omefire closed the pull request at: https://github.com/apache/cordova-lib/pull/139 --- 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

[GitHub] cordova-lib pull request: CB-8226 'cordova platform add' : Look up...

2015-01-02 Thread omefire
GitHub user omefire opened a pull request: https://github.com/apache/cordova-lib/pull/140 CB-8226 'cordova platform add' : Look up version in config.xml if no version specified CB-8226 'cordova platform add' : Look up version in config.xml if no version

[GitHub] cordova-lib pull request: CB-8226 'cordova platform add' : Look up...

2015-01-02 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/140#issuecomment-68585959 These failures seem unrelated to my changes. Ran the tests again on an Ubuntu box and all tests pass. I think these failures are intermittent and sometimes they

[GitHub] cordova-lib pull request: CB-8226 'cordova platform add' : Look up...

2015-01-05 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/140#issuecomment-68743624 Hey @agrieve, I've just addressed your review comments. could you please take a look again ? Thanks. --- If your project is set up for it, you can reply to

[GitHub] cordova-lib pull request: CB-8226 'cordova platform add' : Look up...

2015-01-06 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/140#issuecomment-68909782 Using url.parse introduces additional complexity. Especially, Windows paths are not going to be handled properly: the drive letter gets omitted after parsing. This

[GitHub] cordova-lib pull request: CB-8226 'cordova platform add' : Look up...

2015-01-06 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/140#issuecomment-68916510 Done! 'file://' prefix support dropped. :) --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as wel

[GitHub] cordova-lib pull request: CB-8226 'cordova platform add' : Look up...

2015-01-06 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/140#issuecomment-68942932 Working on it ... --- 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

[GitHub] cordova-lib pull request: CB-8226 'cordova platform add' : Look up...

2015-01-07 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/140#issuecomment-69056950 travis-ci failure not related to my changes. --- 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

[GitHub] cordova-lib pull request: CB-8226 'cordova platform add' : Look up...

2015-01-07 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/140#issuecomment-69095149 @agrieve, have you had a chance to take another look at this ? --Thanks. --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] cordova-lib pull request: CB-8239 Add support for git urls to 'cor...

2015-01-08 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/141#issuecomment-69271082 @hstaudacher, I've added the remaining scenarios. I've copied over some of the changes you made in this branch and merged them with mine.

[GitHub] cordova-lib pull request: CB-8227 CB8237 CB-8238 Add --save option...

2015-01-08 Thread omefire
GitHub user omefire opened a pull request: https://github.com/apache/cordova-lib/pull/144 CB-8227 CB8237 CB-8238 Add --save option to 'cordova platform add', 'cordova platform remove' and 'cordova platform update' CB-8227 CB8237 CB-8238 Add --sa

[GitHub] cordova-cli pull request: CB-8227 CB-8237 CB-8238 Add --save optio...

2015-01-08 Thread omefire
GitHub user omefire opened a pull request: https://github.com/apache/cordova-cli/pull/203 CB-8227 CB-8237 CB-8238 Add --save option to 'cordova platform add', 'cordova platform remove' and 'cordova platform update' CB-8227 CB-8237 CB-8238 Add --save o

[GitHub] cordova-lib pull request: CB-8226 'cordova platform add' : Look up...

2015-01-09 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/140#issuecomment-69377740 @agrieve , I will keep that in mind. Thanks for taking the time to review this ! --- If your project is set up for it, you can reply to this email and have your

[GitHub] cordova-cli pull request: CB-8227 CB-8237 CB-8238 Add --save optio...

2015-01-12 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-cli/pull/203#issuecomment-69664405 @gorkem , plz 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

[GitHub] cordova-lib pull request: CB-8227 CB8237 CB-8238 Add --save option...

2015-01-12 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/144#issuecomment-69664456 @gorkem , plz 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

[GitHub] cordova-lib pull request: CB-8239 Add support for git urls to 'cor...

2015-01-13 Thread omefire
GitHub user omefire opened a pull request: https://github.com/apache/cordova-lib/pull/148 CB-8239 Add support for git urls to 'cordova platform add' CB-8239 Add support for git urls to 'cordova platform add' These changes allow the following scenarios:

[GitHub] cordova-lib pull request: CB-8239 Add support for git urls to 'cor...

2015-01-13 Thread omefire
Github user omefire commented on a diff in the pull request: https://github.com/apache/cordova-lib/pull/148#discussion_r22904360 --- Diff: cordova-lib/spec-cordova/platform.spec.js --- @@ -354,22 +359,94 @@ describe('add function'

[GitHub] cordova-lib pull request: CB-8239 Add support for git urls to 'cor...

2015-01-14 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/148#issuecomment-70015916 Task for docs update : https://issues.apache.org/jira/browse/CB-8308 I'll probably send another PR for that by the end of the week. --- If your project i

[GitHub] cordova-lib pull request: CB-8239 Add support for git urls to 'cor...

2015-01-21 Thread omefire
Github user omefire commented on a diff in the pull request: https://github.com/apache/cordova-lib/pull/148#discussion_r23330721 --- Diff: cordova-lib/src/cordova/lazy_load.js --- @@ -284,4 +286,47 @@ function custom(platforms, platform) { }); } +// Returns

[GitHub] cordova-lib pull request: CB-8239 Add support for git urls to 'cor...

2015-01-21 Thread omefire
Github user omefire commented on a diff in the pull request: https://github.com/apache/cordova-lib/pull/148#discussion_r23330749 --- Diff: cordova-lib/spec-cordova/util.spec.js --- @@ -183,4 +184,138 @@ describe('util module', function() { expect(res.in

[GitHub] cordova-lib pull request: CB-8239 Add support for git urls to 'cor...

2015-01-22 Thread omefire
Github user omefire commented on a diff in the pull request: https://github.com/apache/cordova-lib/pull/148#discussion_r23428777 --- Diff: cordova-lib/src/cordova/platform.js --- @@ -71,102 +72,98 @@ function add(hooksRunner, projectRoot, targets, opts) { // The

[GitHub] cordova-lib pull request: CB-8239 Add support for git urls to 'cor...

2015-01-23 Thread omefire
Github user omefire commented on a diff in the pull request: https://github.com/apache/cordova-lib/pull/148#discussion_r23488114 --- Diff: cordova-lib/src/cordova/util.js --- @@ -262,3 +270,81 @@ function addModuleProperty(module, symbol, modulePath, opt_wrap, opt_obj

[GitHub] cordova-lib pull request: CB-8239 Add support for git urls to 'cor...

2015-01-23 Thread omefire
Github user omefire commented on a diff in the pull request: https://github.com/apache/cordova-lib/pull/148#discussion_r23488149 --- Diff: cordova-lib/src/cordova/util.js --- @@ -55,6 +61,8 @@ exports.preProcessOptions = preProcessOptions; exports.addModuleProperty

[GitHub] cordova-lib pull request: CB-8239 Add support for git urls to 'cor...

2015-01-23 Thread omefire
Github user omefire commented on a diff in the pull request: https://github.com/apache/cordova-lib/pull/148#discussion_r23488733 --- Diff: cordova-lib/spec-cordova/lazy_load.spec.js --- @@ -26,7 +26,8 @@ var lazy_load = require('../src/cordova/lazy_load'),

[GitHub] cordova-lib pull request: CB-8239 Add support for git urls to 'cor...

2015-01-23 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/148#issuecomment-71293539 Tests are passing locally, so is jshint. I don't know why travis isn't picking up my changes. --- If your project is set up for it, you can reply to this

[GitHub] cordova-lib pull request: CB-8227 CB8237 CB-8238 Add --save option...

2015-01-29 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/144#issuecomment-72096263 working on fixing merge conflicts. will update this once that's done. --- If your project is set up for it, you can reply to this email and have your reply a

[GitHub] cordova-lib pull request: CB-8227 CB8237 CB-8238 Add --save option...

2015-02-02 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/144#issuecomment-72536563 @gorkem, I agree with doing the save if config_json has an auto_save option. that's what I was working on adding, unless you want to take it on. :) --- If

[GitHub] cordova-lib pull request: CB-8227 CB8237 CB-8238 Add --save option...

2015-02-02 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/144#issuecomment-72538019 @gorkem I''m planning on adding corresponding PR's for plugins. --- If your project is set up for it, you can reply to this email and have your

[GitHub] cordova-lib pull request: CB-8239 Add support for git urls to 'cor...

2015-02-02 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/141#issuecomment-72557870 I think we don't need this anymore as the functionality this PR wanted to introduce has already been included here : https://github.com/apache/cordova-lib/c

[GitHub] cordova-lib pull request: CB-8239 Add support for git urls to 'cor...

2015-02-03 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/148#issuecomment-72637685 Hey @agrieve, It seems like the refactoring you did before merging this in introduced a bug : Suppose the latest android version is: 3.7.0 and my project

[GitHub] cordova-lib pull request: CB-8406 BugFix: 'cordova platform update...

2015-02-03 Thread omefire
GitHub user omefire opened a pull request: https://github.com/apache/cordova-lib/pull/159 CB-8406 BugFix: 'cordova platform update' should not be upgrading the pl... CB-8406: -'cordova platform update' updates platform to the version in config.xml inst

[GitHub] cordova-lib pull request: CB-8227 CB-8237 CB-8238 Add --save optio...

2015-02-03 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/144#issuecomment-72642865 I have made the following modifications : - added support for autosave as suggested by Gorkem - refactored --save feature to blend into the new code

[GitHub] cordova-lib pull request: CB-8239 Add support for git urls to 'cor...

2015-02-03 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/148#issuecomment-72642965 Here's a pull request for the fix of the afore-mentionned bug : https://github.com/apache/cordova-lib/pull/159 --- If your project is set up for it, you can

[GitHub] cordova-cli pull request: CB-8227 CB-8237 CB-8238 Add --save optio...

2015-02-03 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-cli/pull/203#issuecomment-72644631 @jsoref , thanks! space issue fixed. --- 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

[GitHub] cordova-lib pull request: CB-8227 CB-8237 CB-8238 Add --save optio...

2015-02-03 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/144#issuecomment-72743163 related PR: https://github.com/apache/cordova-cli/pull/203 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as

[GitHub] cordova-cli pull request: CB-8227 CB-8237 CB-8238 Add --save optio...

2015-02-03 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-cli/pull/203#issuecomment-72743227 related PR: https://github.com/apache/cordova-lib/pull/144 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as

[GitHub] cordova-lib pull request: CB-8227 CB-8237 CB-8238 Add --save optio...

2015-02-03 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/144#issuecomment-72790912 appveyor failed because "Build execution time has reached the maximum allowed time for your plan (40 minutes)." --- If your project is set up for it, you

[GitHub] cordova-lib pull request: CB-8227 CB-8237 CB-8238 Add --save optio...

2015-02-04 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/144#issuecomment-72901151 @agrieve and @gorkem, plz 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

[GitHub] cordova-lib pull request: CB-8227 CB-8237 CB-8238 Add --save optio...

2015-02-05 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/144#issuecomment-73092239 @agrieve , sorry. I ran into an issue with git refusing to do a rebase : http://stackoverflow.com/questions/14410421/git-rebase-merge-conflict-cannot-continue

[GitHub] cordova-lib pull request: CB-8227 CB-8237 CB-8238 Add --save optio...

2015-02-05 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/144#issuecomment-73092687 @purplecabbage , yes there were errors in the tests but those were not related to these changes and those tests only occasionally fail in appveyor while travis

[GitHub] cordova-lib pull request: CB-8227 CB-8237 CB-8238 Add --save optio...

2015-02-05 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/144#issuecomment-73156531 Hey @gorkem, the commits have been squashed. --- 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

[GitHub] cordova-lib pull request: CB-8227 CB-8237 CB-8238 Add --save optio...

2015-02-05 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/144#issuecomment-73197538 Hey @gorkem, I just tested 'cordova platform add https://github.com/apache/cordova-android.git --save', and it works for me. did you make sure to use

[GitHub] cordova-lib pull request: CB-8227 CB-8237 CB-8238 Add --save optio...

2015-02-06 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/144#issuecomment-73205488 asdf --- 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

[GitHub] cordova-cli pull request: CB-8439 Fix 'cordova platform update' do...

2015-02-08 Thread omefire
GitHub user omefire opened a pull request: https://github.com/apache/cordova-cli/pull/208 CB-8439 Fix 'cordova platform update' documentation to include CB-8439 Fix 'cordova platform update' documentation to include You can merge this pull request into a Git r

[GitHub] cordova-lib pull request: CB-8420 'cordova plugin add' should retr...

2015-02-09 Thread omefire
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 o

[GitHub] cordova-lib pull request: CB-8420 'cordova plugin add' should retr...

2015-02-09 Thread omefire
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

[GitHub] cordova-lib pull request: CB-8227 CB-8237 CB-8238 Add --save optio...

2015-02-09 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/144#issuecomment-73621340 Hey @gorkem, I'm finalising a PR to remove the save command and add/update the tests. --- If your project is set up for it, you can reply to this email and

[GitHub] cordova-lib pull request: CB-8420 'cordova plugin add' should retr...

2015-02-09 Thread omefire
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 :

[GitHub] cordova-lib pull request: CB-8420 'cordova plugin add' should retr...

2015-02-09 Thread omefire
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

[GitHub] cordova-lib pull request: CB-8420 'cordova plugin add' should retr...

2015-02-11 Thread omefire
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

[GitHub] cordova-lib pull request: Save/Restore for plugins and platforms

2015-02-12 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/166#issuecomment-74128449 Cool feature list. reviewing 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

[GitHub] cordova-lib pull request: Save/Restore for plugins and platforms

2015-02-12 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/166#issuecomment-74143188 if no version is specified for (e.g: cordova plugin add org.apache.cordova.device --save), we end up saving the current edge version (org.apache.cordova.device

[GitHub] cordova-lib pull request: Save/Restore for plugins and platforms

2015-02-12 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/166#issuecomment-74145216 I think the 'saving to config.xml' should be moved into a separate function and called from here. this function is getting way too long to easily reason

[GitHub] cordova-lib pull request: Save/Restore for plugins and platforms

2015-02-12 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/166#issuecomment-74146324 'saving to config.xml' should happen after the plugin has been installed on the platforms. That way, if any error is encountered during the in

[GitHub] cordova-lib pull request: Save/Restore for plugins and platforms

2015-02-12 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/166#issuecomment-74151061 Hey @gorkem, I notice that --save covers add and remove. Are you going to send a pull request to handle the update case ? : 'cordova plugin u

[GitHub] cordova-lib pull request: Save/Restore for plugins and platforms

2015-02-12 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/166#issuecomment-74151973 Hey @gorkem, I notice that --save covers add and remove. Are you going to send a pull request to handle the update case ? : 'cordova plugin u

[GitHub] cordova-lib pull request: Save/Restore for plugins and platforms

2015-02-12 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/166#issuecomment-74155732 When the flag --save/autosave on, we should not be retrieving the version from config.xml. For example, - config.xml contains org.apache.cordova.device

[GitHub] cordova-lib pull request: Save/Restore for plugins and platforms

2015-02-13 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/166#issuecomment-74293656 Also, we should probably save the plugin variables along with the version into config.xml, so that on prepare/restore, we don't fail. --- If your project is s

[GitHub] cordova-lib pull request: Save/Restore for plugins and platforms

2015-02-13 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/166#issuecomment-74313313 hmmm, it seems to me that during prepare, while restoring from config.xml, we do use variables from config.xml. so, we should save it during --save/autosave. Please

[GitHub] cordova-lib pull request: Save/Restore for plugins and platforms

2015-02-13 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/166#issuecomment-74318530 What if a plugin install fails ? don't we want to continue with other plugins' install like we do for platforms ? That way, we also handle platform

[GitHub] cordova-lib pull request: CB-8482: Update engine syntax within con...

2015-02-16 Thread omefire
GitHub user omefire opened a pull request: https://github.com/apache/cordova-lib/pull/168 CB-8482: Update engine syntax within config.xml CB-8482: Update engine syntax within config.xml You can merge this pull request into a Git repository by running: $ git pull https

[GitHub] cordova-lib pull request: CB-8482: Update engine syntax within con...

2015-02-17 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/168#issuecomment-74720162 CB-8482: Update engine syntax within config.xml - from : `` - to : `` --- If your project is set up for it, you can reply to this email and have

[GitHub] cordova-lib pull request: CB-8499 `cordova platform save` should s...

2015-02-20 Thread omefire
GitHub user omefire opened a pull request: https://github.com/apache/cordova-lib/pull/170 CB-8499 `cordova platform save` should save all currently installed platforms and their versions/git-url/folder-location into config.xml. CB-8499 `cordova platform save` should save all

[GitHub] cordova-lib pull request: CB-8499 `cordova platform save` should s...

2015-02-20 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/170#issuecomment-75298325 @agrieve , @vladimir-kotikov 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

[GitHub] cordova-lib pull request: CB-8499 `cordova platform save` should s...

2015-02-20 Thread omefire
Github user omefire commented on a diff in the pull request: https://github.com/apache/cordova-lib/pull/170#discussion_r25098780 --- Diff: cordova-lib/src/cordova/platform_metadata.js --- @@ -0,0 +1,113 @@ +/** +Licensed to the Apache Software Foundation (ASF) under one

[GitHub] cordova-lib pull request: CB-8499 `cordova platform save` should s...

2015-02-20 Thread omefire
Github user omefire commented on a diff in the pull request: https://github.com/apache/cordova-lib/pull/170#discussion_r25098622 --- Diff: cordova-lib/src/cordova/platform_metadata.js --- @@ -0,0 +1,113 @@ +/** +Licensed to the Apache Software Foundation (ASF) under one

[GitHub] cordova-lib pull request: CB-8499 `cordova platform save` should s...

2015-02-20 Thread omefire
Github user omefire commented on a diff in the pull request: https://github.com/apache/cordova-lib/pull/170#discussion_r25100140 --- Diff: cordova-lib/src/cordova/platform.js --- @@ -174,6 +181,24 @@ function addHelper(cmd, hooksRunner, projectRoot, targets, opts

[GitHub] cordova-lib pull request: CB-8499 `cordova platform save` should s...

2015-02-20 Thread omefire
Github user omefire commented on a diff in the pull request: https://github.com/apache/cordova-lib/pull/170#discussion_r25100062 --- Diff: cordova-lib/src/cordova/platform.js --- @@ -174,6 +181,24 @@ function addHelper(cmd, hooksRunner, projectRoot, targets, opts

[GitHub] cordova-lib pull request: CB-8499 `cordova platform save` should s...

2015-02-20 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/170#issuecomment-75314035 @vladimir-kotikov Thanks for reviewing, All your reviews have been addressed, please take another look. --- If your project is set up for it, you can reply to this

[GitHub] cordova-lib pull request: CB-8482: Update engine syntax within con...

2015-02-20 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/168#issuecomment-75346177 There will need to be another pass to update dependent modules to handle the possibility that some new code depending on old syntax might have been inserted in the

[GitHub] cordova-lib pull request: Save/Restore for plugins and platforms

2015-02-23 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/166#issuecomment-75574968 Reviewing latest changes ... --- 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

[GitHub] cordova-lib pull request: Save/Restore for plugins and platforms

2015-02-23 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/166#issuecomment-75615699 you forgot to negate opts.save as well : if(!autosave && !opts.save && ...) --- If your project is set up for it, you can reply to this email and

[GitHub] cordova-lib pull request: CB-8499 `cordova platform save` should s...

2015-02-23 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/170#issuecomment-75623519 @gorkem, yes platforms.json is a new file in the platforms directory. think of it as the equivalent of the .fetch.json file we have for plugins. --- If your

[GitHub] cordova-lib pull request: Save/Restore for plugins and platforms

2015-02-23 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/166#issuecomment-75640710 when --save/autosave is on, we should be overriding what's in config.xml, like we do on the platform side : https://github.com/apache/cordova-lib/blob/m

[GitHub] cordova-lib pull request: Save/Restore for plugins and platforms

2015-02-23 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/166#issuecomment-75656828 what about the other options ? (opts). shouldn't we pass them along to 'prepare' ? --- If your project is set up for it, you can reply to this

[GitHub] cordova-lib pull request: CB-8482: Update engine syntax within con...

2015-02-23 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/168#issuecomment-75660844 @gorkem, cool. working on a rebase + update to new dependent modules ... --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] cordova-lib pull request: CB-8482: Update engine syntax within con...

2015-02-23 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/168#issuecomment-75669608 @gorkem, I did find some things that needed to be updated in restore-util.js. those have been taken care of. --- If your project is set up for it, you can reply to

[GitHub] cordova-lib pull request: CB-8499 `cordova platform save` should s...

2015-02-23 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/170#issuecomment-75670437 I think this PR is ready to be merged in. --- 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

[GitHub] cordova-lib pull request: CB-8578 BugFix: 'cordova plugin ...

2015-02-27 Thread omefire
GitHub user omefire opened a pull request: https://github.com/apache/cordova-lib/pull/173 CB-8578 BugFix: 'cordova plugin should be able to restore urls and folders in addition to versions. CB-8578 BugFix: 'cordova plugin should be able to restore urls and folders in a

[GitHub] cordova-lib pull request: CB-8577 BugFix 'cordova plugin add

2015-02-27 Thread omefire
GitHub user omefire opened a pull request: https://github.com/apache/cordova-lib/pull/174 CB-8577 BugFix 'cordova plugin add --save' should properly save variables into config.xml to enable restoring later on. CB-8577 BugFix 'cordova plugin add --save'

[GitHub] cordova-lib pull request: CB-8577 BugFix 'cordova plugin add

2015-02-27 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/174#issuecomment-76512869 What do you mean ? --- 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

[GitHub] cordova-lib pull request: CB-8577 BugFix 'cordova plugin add

2015-02-28 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/174#issuecomment-76532251 @gorkem , I disagree with your statement that it does not affect functionality. As it currently stands, the code doesn't work for plugins that have variable

[GitHub] cordova-lib pull request: CB-8577 BugFix 'cordova plugin add

2015-02-28 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/174#issuecomment-76532764 It currently doesn't work in 100% of the cases but I do agree we can postpone the fix. I don't really care what the name of the element

[GitHub] cordova-lib pull request: CB-8577 BugFix 'cordova plugin add

2015-02-28 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/174#issuecomment-76533060 It currently doesn't work in 100% of the cases but I do agree we can postpone the fix. It doesn't really matter what the name of the element

[GitHub] cordova-lib pull request: CB-8577 BugFix 'cordova plugin add

2015-03-01 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/174#issuecomment-76641117 Fixing it in Config.getFeature(id) would mean that we would have to differentiate between `` types : variables and non-variables. I think that could be unecessarily

[GitHub] cordova-lib pull request: CB-8577 BugFix 'cordova plugin add

2015-03-01 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/174#issuecomment-76641530 Fixing it in Config.getFeature(id) would mean that we would have to differentiate between `` types : variables and non-variables. I think that would be unecessarily

[GitHub] cordova-lib pull request:

2015-03-01 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/commit/4c0fec7d30df238cfc4c834a4235b2c8fcf330ad#commitcomment-9979467 @gorkem, this change introduces another bug : the variables list now contains all the `` such as 'id', 'url

[GitHub] cordova-lib pull request: CB-8577 BugFix 'cordova plugin add

2015-03-01 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/174#issuecomment-76663106 @gorkem, the change you've committed to fix this issue ( https://github.com/apache/cordova-lib/commit/4c0fec7d30df238cfc4c834a4235b2c8fcf330ad ) introduces an

[GitHub] cordova-lib pull request: CB-8499 `cordova platform save` should s...

2015-03-02 Thread omefire
Github user omefire commented on a diff in the pull request: https://github.com/apache/cordova-lib/pull/170#discussion_r25626517 --- Diff: cordova-lib/src/cordova/platform_metadata.js --- @@ -0,0 +1,102 @@ +/** +Licensed to the Apache Software Foundation (ASF) under one

[GitHub] cordova-lib pull request: CB-8499 `cordova platform save` should s...

2015-03-02 Thread omefire
Github user omefire commented on a diff in the pull request: https://github.com/apache/cordova-lib/pull/170#discussion_r25626571 --- Diff: cordova-lib/src/cordova/platform_metadata.js --- @@ -0,0 +1,102 @@ +/** +Licensed to the Apache Software Foundation (ASF) under one

[GitHub] cordova-lib pull request: CB-8499 `cordova platform save` should s...

2015-03-02 Thread omefire
Github user omefire commented on a diff in the pull request: https://github.com/apache/cordova-lib/pull/170#discussion_r25652223 --- Diff: cordova-lib/src/cordova/platform_metadata.js --- @@ -0,0 +1,102 @@ +/** +Licensed to the Apache Software Foundation (ASF) under one

[GitHub] cordova-lib pull request: CB-8499 `cordova platform save` should s...

2015-03-03 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/170#issuecomment-77011164 @agrieve , I addressed your comments. could you please take another look ? --- If your project is set up for it, you can reply to this email and have your reply

[GitHub] cordova-lib pull request: CB-8596 Expose APIs to retrieve platform...

2015-03-03 Thread omefire
GitHub user omefire opened a pull request: https://github.com/apache/cordova-lib/pull/177 CB-8596 Expose APIs to retrieve platforms and plugins saved in config.xml CB-8596 Expose APIs to retrieve platforms and plugins saved in config.xml so as to abstract clients away from the

[GitHub] cordova-lib pull request: CB-8596 Expose APIs to retrieve platform...

2015-03-03 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/177#issuecomment-77107166 After migrating platforms & plugins to package.json, This implementation will be updated, but clients will remain unchanged. --- If your project is set up fo

[GitHub] cordova-lib pull request: CB-8596 Expose APIs to retrieve platform...

2015-03-03 Thread omefire
Github user omefire commented on the pull request: https://github.com/apache/cordova-lib/pull/177#issuecomment-77107191 @agrieve , @stevengill 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

  1   2   3   4   5   6   7   >