[GitHub] cordova-browser pull request: CB-8206: Browser platform: Add suppo...
Github user asfgit closed the pull request at: https://github.com/apache/cordova-browser/pull/6 --- 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user nikhilkh commented on the pull request: https://github.com/apache/cordova-browser/pull/6#issuecomment-71228160 @jsoref Updated the commit message. Please review and help merge to master. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user jsoref commented on the pull request: https://github.com/apache/cordova-browser/pull/6#issuecomment-71222792 ok, one last final thing: * Please amend the commit message to not have the `:` after the `CB-8206` (I'm fairly confident this is documented in our instructions). Then, someone can push this to cordova-browser. --- 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user mmocny commented on the pull request: https://github.com/apache/cordova-browser/pull/6#issuecomment-71211172 (Josh I'll let you have the last word on landing, you have been running 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user mmocny commented on the pull request: https://github.com/apache/cordova-browser/pull/6#issuecomment-71210993 ..Thanks! --- 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user mmocny commented on the pull request: https://github.com/apache/cordova-browser/pull/6#issuecomment-71210975 I reviewed this in a previous iteration, and the latest changes look fine. Since this feature does not exist yet anyway, and I'm not too concerned about inline updates to browser platform, I think this is more than ready to ship. --- 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user nikhilkh commented on the pull request: https://github.com/apache/cordova-browser/pull/6#issuecomment-71115226 @jsoref Your comments have been addressed. I will wait for someone else to review this. A checklist would be great - looks like there is a doc here: https://github.com/apache/cordova-coho/blob/master/docs/code-reviews.md but lacks details. --- 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-browser/pull/6#discussion_r23417228
--- Diff: bin/update ---
@@ -0,0 +1,33 @@
+#!/usr/bin/env node
+
+/*
+ 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 update = require('./lib/update');
+
+// check for help flag
+if (['--help', '/?', '-h', 'help', '-help',
'/help'].indexOf(process.argv[2]) > -1) {
+update.help();
+} else {
+update.run(process.argv).done(function () {
+console.log('Successfully updated browser project.');
+}, function (err) {
+console.error('Update failed due to', err);
+process.exit(2);
+});
+}
--- End diff --
Sorry that I did not update all the files correctly earlier. It would be
great to automate something like 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-browser/pull/6#discussion_r23417160
--- Diff: bin/lib/create.js ---
@@ -75,7 +69,7 @@ exports.createProject =
function(project_path,package_name,project_name){
'run',
'build',
'clean',
-'version',
+'version'
--- End diff --
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 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user jsoref commented on the pull request: https://github.com/apache/cordova-browser/pull/6#issuecomment-71108929 I'd like someone else to review this -- but please address my other comments. Sorry for the multipass review. And, I need to see about ensuring there's a checklist (on the wiki or in coho or something) for most of these points... --- 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user jsoref commented on a diff in the pull request:
https://github.com/apache/cordova-browser/pull/6#discussion_r23414343
--- Diff: bin/update ---
@@ -0,0 +1,33 @@
+#!/usr/bin/env node
+
+/*
+ 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 update = require('./lib/update');
+
+// check for help flag
+if (['--help', '/?', '-h', 'help', '-help',
'/help'].indexOf(process.argv[2]) > -1) {
+update.help();
+} else {
+update.run(process.argv).done(function () {
+console.log('Successfully updated browser project.');
+}, function (err) {
+console.error('Update failed due to', err);
+process.exit(2);
+});
+}
--- End diff --
please ensure that there's a new line at the end of each file, when you see
a red null/carriage mark w/ a green background, that means that the diff system
is pointing out that you're missing the newline.
---
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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user jsoref commented on a diff in the pull request:
https://github.com/apache/cordova-browser/pull/6#discussion_r23414226
--- Diff: bin/lib/create.js ---
@@ -75,7 +69,7 @@ exports.createProject =
function(project_path,package_name,project_name){
'run',
'build',
'clean',
-'version',
+'version'
--- End diff --
you shouldn't make this change. trailing commas are legal and in fact
encouraged for arrays, as they make diffs easier to read.
---
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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user nikhilkh commented on the pull request: https://github.com/apache/cordova-browser/pull/6#issuecomment-71106847 Looking for anyone who can review this and commit it? It's quite a simple change. --- 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user nikhilkh commented on the pull request: https://github.com/apache/cordova-browser/pull/6#issuecomment-70004026 @jsoref Thanks for reviewing this and the instructions for doing the git magic. It was useful. I have made the changes that you requested. --- 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user nikhilkh commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/6#discussion_r22961441 --- Diff: node_modules/.bin/shjs --- @@ -1 +1,15 @@ -../shelljs/bin/shjs --- End diff -- Good question @mmocny and you're not the first one to ask that: http://callback.markmail.org/thread/vl35coc24d4vjdph. Cordova-windows also does this. I tend to agree with you that this is less than ideal design. As mentioned in the thread, there is a couple of technical limitations currently and @jsoref expressed some other concerns. --- 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user jsoref commented on the pull request: https://github.com/apache/cordova-browser/pull/6#issuecomment-69970475 well, err, specifically: for lines you're adding in aad4c25 with bad whitespace, you should fold in the fixes from 2d77f2b. If you're fixing the entire file, you can do that as a distinct commit. --- 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user mmocny commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/6#discussion_r22960109 --- Diff: node_modules/.bin/shjs --- @@ -1 +1,15 @@ -../shelljs/bin/shjs --- End diff -- For offline installs you still need to pre-fetch, where you can do the npm install step as well. And I'm not at this point discussing what does into an RC, just what's stored in our git master. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user jsoref commented on the pull request: https://github.com/apache/cordova-browser/pull/6#issuecomment-69970299 @nikhilkh: please use `git rebase` to fold the fixes for 2d77f2b into aad4c25 --- 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user jsoref commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/6#discussion_r22960012 --- Diff: node_modules/.bin/shjs --- @@ -1 +1,15 @@ -../shelljs/bin/shjs --- End diff -- @nikhilkh : i would prefer you split it into a distinct commit (before this commit) -- see http://stackoverflow.com/questions/4307095/git-how-to-split-up-a-commit-buried-in-history for how to do 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 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user jsoref commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/6#discussion_r22959941 --- Diff: node_modules/.bin/shjs --- @@ -1 +1,15 @@ -../shelljs/bin/shjs --- End diff -- @mmocny : checking them in is the only way things actually work, otherwise offline installs and various other things break. blackberry10 checks in its node_modules too --- 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user mmocny commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/6#discussion_r22957388 --- Diff: node_modules/.bin/shjs --- @@ -1 +1,15 @@ -../shelljs/bin/shjs --- End diff -- Why are node_modules even checked in? Shouldn't they just be added to .gitignore? I double checked, and the only other cordova repo that checks these in is firefoxos, which I think is also an oversight. --- 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user nikhilkh commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/6#discussion_r22911133 --- Diff: node_modules/.bin/shjs --- @@ -1 +1,15 @@ -../shelljs/bin/shjs --- End diff -- This is done automagically by node on windows to create a shim for a *nix batch file: http://stackoverflow.com/questions/10396305/npm-package-bin-script-for-windows. While I agree with you that this is not strictly related to `update` change, since this is in the `node_modules` directory, I would prefer to keep this as is. Let me know if you still disagree. --- 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user purplecabbage commented on a diff in the pull request:
https://github.com/apache/cordova-browser/pull/6#discussion_r22905510
--- Diff: bin/lib/update.js ---
@@ -0,0 +1,59 @@
+/*
+ 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'),
+create = require('./create'),
+fs = require('fs'),
+shell = require('shelljs');
+
+module.exports.help = function () {
+console.log("WARNING : Make sure to back up your project before
updating!");
+console.log("Usage: update PathToProject ");
+console.log("PathToProject : The path the project you would like
to update.");
+console.log("examples:");
+console.log("update C:\\Users\\anonymous\\Desktop\\MyProject");
+};
+
+module.exports.run = function (argv) {
+var projectPath = argv[2];
+if (!fs.existsSync(projectPath)) {
+// if specified project path is not valid then reject promise
+Q.reject("Browser platform does not exist here: " + projectPath);
+}
+return Q().then(function() {
+console.log("Removing existing browser platform.");
+shellfatal(shell.rm, '-rf', projectPath);
+create.createProject(projectPath);
+});
+}
+
+function shellfatal(shellFunc)
+{
+var slicedArgs = Array.prototype.slice.call(arguments, 1);
+try
+{
+shell.config.fatal = true;
+var returnVal = shellFunc.apply(shell, slicedArgs);
+}
+finally
+{
+shell.config.fatal = false;
+}
+return returnVal;
+}
--- End diff --
http://stackoverflow.com/questions/729692/why-should-files-end-with-a-newline
Generally a best practice, not just js, and not just this project.
---
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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-browser/pull/6#discussion_r22905187
--- Diff: bin/lib/update.js ---
@@ -0,0 +1,59 @@
+/*
+ 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'),
+create = require('./create'),
+fs = require('fs'),
+shell = require('shelljs');
+
+module.exports.help = function () {
+console.log("WARNING : Make sure to back up your project before
updating!");
+console.log("Usage: update PathToProject ");
+console.log("PathToProject : The path the project you would like
to update.");
+console.log("examples:");
+console.log("update C:\\Users\\anonymous\\Desktop\\MyProject");
+};
+
+module.exports.run = function (argv) {
+var projectPath = argv[2];
+if (!fs.existsSync(projectPath)) {
+// if specified project path is not valid then reject promise
+Q.reject("Browser platform does not exist here: " + projectPath);
+}
+return Q().then(function() {
+console.log("Removing existing browser platform.");
+shellfatal(shell.rm, '-rf', projectPath);
+create.createProject(projectPath);
+});
+}
+
+function shellfatal(shellFunc)
+{
+var slicedArgs = Array.prototype.slice.call(arguments, 1);
+try
+{
+shell.config.fatal = true;
+var returnVal = shellFunc.apply(shell, slicedArgs);
+}
+finally
+{
+shell.config.fatal = false;
+}
+return returnVal;
+}
--- End diff --
Interesting! I will fix this. I'm curious - what's the rationale behind
this? Is this just a styling best practice on this project or in JavaScript
community?
---
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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-browser/pull/6#discussion_r22905021
--- Diff: bin/lib/create.js ---
@@ -26,14 +26,11 @@ var fs = require('fs'),
ROOT= path.join(__dirname, '..', '..'),
check_reqs = require('./check_reqs');
-
exports.createProject = function(project_path,package_name,project_name){
var VERSION = fs.readFileSync(path.join(ROOT, 'VERSION'), 'utf-8');
// Set default values for path, package and name
project_path = typeof project_path !== 'undefined' ? project_path :
"CordovaExample";
-package_name = typeof package_name !== 'undefined' ? package_name :
'org.apache.cordova.example';
--- End diff --
Turns out that `update` depends on `create`. As I mentioned in my pull
request, I had to remove these console.log messages as for the browser platform
we do not save the package and project name on creation. On upgrade, I do not
have a package and project name when I invoke `createProject`. These messages
had marginal value so I decided to remove them.
---
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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user jsoref commented on a diff in the pull request: https://github.com/apache/cordova-browser/pull/6#discussion_r22899820 --- Diff: node_modules/.bin/shjs --- @@ -1 +1,15 @@ -../shelljs/bin/shjs --- End diff -- this change looks like you replaced a *nix symlink w/ a file. the change shouldn't be done in the same commit as adding support for `update`. and you should probably do something to ensure that someone doesn't later undo this when working from *nix. --- 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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user jsoref commented on a diff in the pull request:
https://github.com/apache/cordova-browser/pull/6#discussion_r22899501
--- Diff: bin/lib/create.js ---
@@ -26,14 +26,11 @@ var fs = require('fs'),
ROOT= path.join(__dirname, '..', '..'),
check_reqs = require('./check_reqs');
-
exports.createProject = function(project_path,package_name,project_name){
var VERSION = fs.readFileSync(path.join(ROOT, 'VERSION'), 'utf-8');
// Set default values for path, package and name
project_path = typeof project_path !== 'undefined' ? project_path :
"CordovaExample";
-package_name = typeof package_name !== 'undefined' ? package_name :
'org.apache.cordova.example';
--- End diff --
this looks suspiciously like you saved after updating and overwrote someone
else's changes.
in general, adding a feature (like `update`) shouldn't have any impact to a
parallel command (like `create`)
---
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-browser pull request: CB-8206: Browser platform: Add suppo...
Github user jsoref commented on a diff in the pull request:
https://github.com/apache/cordova-browser/pull/6#discussion_r22899370
--- Diff: bin/lib/update.js ---
@@ -0,0 +1,59 @@
+/*
+ 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'),
+create = require('./create'),
+fs = require('fs'),
+shell = require('shelljs');
+
+module.exports.help = function () {
+console.log("WARNING : Make sure to back up your project before
updating!");
+console.log("Usage: update PathToProject ");
+console.log("PathToProject : The path the project you would like
to update.");
+console.log("examples:");
+console.log("update C:\\Users\\anonymous\\Desktop\\MyProject");
+};
+
+module.exports.run = function (argv) {
+var projectPath = argv[2];
+if (!fs.existsSync(projectPath)) {
+// if specified project path is not valid then reject promise
+Q.reject("Browser platform does not exist here: " + projectPath);
+}
+return Q().then(function() {
+console.log("Removing existing browser platform.");
+shellfatal(shell.rm, '-rf', projectPath);
+create.createProject(projectPath);
+});
+}
+
+function shellfatal(shellFunc)
+{
+var slicedArgs = Array.prototype.slice.call(arguments, 1);
+try
+{
+shell.config.fatal = true;
+var returnVal = shellFunc.apply(shell, slicedArgs);
+}
+finally
+{
+shell.config.fatal = false;
+}
+return returnVal;
+}
--- End diff --
The red mark here means that you didn't insert a newline at the end of your
file. please do insert one.
---
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-browser pull request: CB-8206: Browser platform: Add suppo...
GitHub user nikhilkh opened a pull request: https://github.com/apache/cordova-browser/pull/6 CB-8206: Browser platform: Add support for update This change add supports for updating the browser platform. I ended up deleting a couple of log messages from create as there is no way to retrieve and log them on update. You can merge this pull request into a Git repository by running: $ git pull https://github.com/MSOpenTech/cordova-browser CB-8206 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/cordova-browser/pull/6.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 #6 commit aad4c25d489468df32b871fd00ec4a1f64d9a34d Author: Nikhil Khandelwal Date: 2014-12-19T01:14:18Z CB-8206: Browser platform: Add support for update --- 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
