You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by nikhilkh <gi...@git.apache.org> on 2015/05/16 02:13:04 UTC
[GitHub] cordova-coho pull request: Add command to merge PR
GitHub user nikhilkh opened a pull request:
https://github.com/apache/cordova-coho/pull/78
Add command to merge PR
Merging github PR the 'right' way can require a bunch of commands. This change automates that - given the PR number and github remote name.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MSOpenTech/cordova-coho merge_pr
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-coho/pull/78.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 #78
----
commit 12c032365d3d80562ceaf1f5a3469eafb8544e91
Author: Nikhil Khandelwal <ni...@microsoft.com>
Date: 2015-05-15T23:58:17Z
Add command to merge 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 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-coho pull request: Add command to merge PR
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-coho/pull/78#discussion_r30698930
--- Diff: src/merge-pr.js ---
@@ -0,0 +1,62 @@
+
+var flagutil = require('./flagutil');
+var optimist = require('optimist');
+var shelljs = require('shelljs');
+var executil = require('./executil');
+var gitutil = require('./gitutil');
+var superspawn = require('./superspawn');
+
+module.exports = function *(argv) {
+ var opt = flagutil.registerHelpFlag(optimist);
+ opt.options('pr', {
+ desc: 'PR # that needs to be merged',
+ demand: true
+ }).options('remote', {
+ desc: 'Named remote for the github apache mirror',
+ demand: true
+ }).options('pretend', {
+ desc: 'Don\'t actually run commands, just print what would be run.',
+ type:'boolean'
+ });
+ argv = opt
+ .usage('Merges the pull request to master\n' +
+ '\n' +
+ 'Usage: $0 merge-pr --pr 111 --remote mirror')
+ .argv;
+ if (argv.h) {
+ optimist.showHelp();
+ process.exit(1);
+ }
+ yield gitutil.stashAndPop("", function*() {
+ yield executil.execOrPretend(executil.ARGS('git checkout master'), argv.pretend);
+
+ yield executil.execOrPretend(executil.ARGS('git pull origin master'), argv.pretend);
+
+ yield executil.execOrPretend(['git', 'fetch', '-fu', argv.remote,
--- End diff --
Added a comment.
---
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-coho pull request: Add command to merge PR
Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on the pull request:
https://github.com/apache/cordova-coho/pull/78#issuecomment-103934974
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-coho pull request: Add command to merge PR
Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on the pull request:
https://github.com/apache/cordova-coho/pull/78#issuecomment-103703256
Awesome addition!
---
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-coho pull request: Add command to merge PR
Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:
https://github.com/apache/cordova-coho/pull/78#discussion_r30717099
--- Diff: src/executil.js ---
@@ -52,7 +52,7 @@ function execHelper(cmdAndArgs, silent, allowError) {
var result = superspawn.spawn(cmdAndArgs[0], cmdAndArgs.slice(1), {stdio: (silent && (silent !== 2)) ? 'default' : 'inherit'});
return result.then(null, function(e) {
if (allowError) {
- return null;
+ throw e;
--- End diff --
This is much nicer :+1:
---
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-coho pull request: Add command to merge PR
Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:
https://github.com/apache/cordova-coho/pull/78#discussion_r30661704
--- Diff: src/merge-pr.js ---
@@ -0,0 +1,62 @@
+
+var flagutil = require('./flagutil');
+var optimist = require('optimist');
+var shelljs = require('shelljs');
+var executil = require('./executil');
+var gitutil = require('./gitutil');
+var superspawn = require('./superspawn');
+
+module.exports = function *(argv) {
+ var opt = flagutil.registerHelpFlag(optimist);
+ opt.options('pr', {
+ desc: 'PR # that needs to be merged',
+ demand: true
+ }).options('remote', {
+ desc: 'Named remote for the github apache mirror',
+ demand: true
+ }).options('pretend', {
+ desc: 'Don\'t actually run commands, just print what would be run.',
+ type:'boolean'
+ });
+ argv = opt
+ .usage('Merges the pull request to master\n' +
+ '\n' +
+ 'Usage: $0 merge-pr --pr 111 --remote mirror')
+ .argv;
+ if (argv.h) {
+ optimist.showHelp();
+ process.exit(1);
+ }
+ yield gitutil.stashAndPop("", function*() {
+ yield executil.execOrPretend(executil.ARGS('git checkout master'), argv.pretend);
+
+ yield executil.execOrPretend(executil.ARGS('git pull origin master'), argv.pretend);
+
+ yield executil.execOrPretend(['git', 'fetch', '-fu', argv.remote,
+ 'refs/pull/' + argv.pr + '/head:pr/' + argv.pr], argv.pretend);
+
+ try {
+ yield executil.execHelper(executil.ARGS('git merge --ff-only pr/' + argv.pr),
+ /*silent*/ true, /*allowError*/ true, argv.pretend);
+ } catch (e) {
+ if (e.message.indexOf('fatal: Not possible to fast-forward, aborting.') > 0) {
+ // Let's try to rebase
+ yield executil.execOrPretend(executil.ARGS('git checkout pr/' + argv.pr), argv.pretend);
+
+ yield executil.execOrPretend(executil.ARGS('git pull --rebase origin master'), argv.pretend);
+
+ yield executil.execOrPretend(executil.ARGS('git checkout master'), argv.pretend);
+
+ yield executil.execOrPretend(executil.ARGS('git merge --ff-only pr/' + argv.pr), argv.pretend);
+
+ var commitMessage = yield executil.execOrPretend(executil.ARGS('git log --format=%B -n 1 HEAD'),
+ argv.pretend);
+
+ yield executil.execOrPretend(['git', 'commit', '--amend', '-m',
--- End diff --
For the case of multiple commits, it would also be a good idea to end with a "git rebase -i" so commits & commit messages can be 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 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-coho pull request: Add command to merge PR
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on the pull request:
https://github.com/apache/cordova-coho/pull/78#issuecomment-103968703
My latest commit removes these class of errors with not specifying the correct remote & 'origin' remote. It implements @agrieve suggestion of auto-detecting the repo/remote from the CWD.
I'm merging this now.
---
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-coho pull request: Add command to merge PR
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on the pull request:
https://github.com/apache/cordova-coho/pull/78#issuecomment-103881305
Thanks, @agrieve & @dblotsky for the review. I have made some changes based on feedback and added a summary message listing commits that were merged at the end.
---
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-coho pull request: Add command to merge PR
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on the pull request:
https://github.com/apache/cordova-coho/pull/78#issuecomment-103970568
Merged with e337cab.
---
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-coho pull request: Add command to merge PR
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-coho/pull/78#discussion_r30698765
--- Diff: src/repo-update.js ---
@@ -129,8 +129,9 @@ function *updateRepos(repos, branches, noFetch) {
yield gitutil.gitCheckout(branchName);
var ret = yield executil.execHelper(executil.ARGS('git merge --ff-only', repo.remoteName + '/' + branchName), false, true);
if (ret === null) {
- ret = yield executil.execHelper(executil.ARGS('git rebase ' + repo.remoteName + '/' + branchName), false, true);
- if (ret === null) {
+ try {
+ ret = yield executil.execHelper(executil.ARGS('git rebase ' + repo.remoteName + '/' + branchName), false, true);
--- 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 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-coho pull request: Add command to merge PR
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-coho/pull/78#discussion_r30577529
--- Diff: src/merge-pr.js ---
@@ -0,0 +1,62 @@
+
+var flagutil = require('./flagutil');
+var optimist = require('optimist');
+var shelljs = require('shelljs');
+var executil = require('./executil');
+var gitutil = require('./gitutil');
+var superspawn = require('./superspawn');
+
+module.exports = function *(argv) {
+ var opt = flagutil.registerHelpFlag(optimist);
+ opt.options('pr', {
+ desc: 'PR # that needs to be merged',
+ demand: true
+ }).options('remote', {
+ desc: 'Named remote for the github apache mirror',
+ demand: true
+ }).options('pretend', {
+ desc: 'Don\'t actually run commands, just print what would be run.',
+ type:'boolean'
+ });
+ argv = opt
+ .usage('Merges the pull request to master\n' +
+ '\n' +
+ 'Usage: $0 merge-pr --pr 111 --remote mirror')
+ .argv;
+ if (argv.h) {
+ optimist.showHelp();
+ process.exit(1);
+ }
+ yield gitutil.stashAndPop("", function*() {
+ yield executil.execOrPretend(executil.ARGS('git checkout master'), argv.pretend);
+
+ yield executil.execOrPretend(executil.ARGS('git pull origin master'), argv.pretend);
+
+ yield executil.execOrPretend(['git', 'fetch', '-fu', argv.remote,
+ 'refs/pull/' + argv.pr + '/head:pr/' + argv.pr], argv.pretend);
+
+ try {
+ yield executil.execHelper(executil.ARGS('git merge --ff-only pr/' + argv.pr),
--- End diff --
It might be more clear to assign `'pr/' + argv.pr` to a variable, since it's used in many places.
---
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-coho pull request: Add command to merge PR
Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:
https://github.com/apache/cordova-coho/pull/78#discussion_r30661261
--- Diff: src/merge-pr.js ---
@@ -0,0 +1,62 @@
+
+var flagutil = require('./flagutil');
+var optimist = require('optimist');
+var shelljs = require('shelljs');
+var executil = require('./executil');
+var gitutil = require('./gitutil');
+var superspawn = require('./superspawn');
+
+module.exports = function *(argv) {
+ var opt = flagutil.registerHelpFlag(optimist);
+ opt.options('pr', {
+ desc: 'PR # that needs to be merged',
+ demand: true
+ }).options('remote', {
--- End diff --
would be much slicker if `remote` was figured out automatically. I *think* this info is already in list-pulls.js, so could maybe get it from 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 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-coho pull request: Add command to merge PR
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-coho/pull/78#discussion_r30698749
--- Diff: src/gitutil.js ---
@@ -43,12 +43,14 @@ exports.tagExists = function*(tagName) {
}
exports.retrieveCurrentBranchName = function*(allowDetached) {
- var ref = yield executil.execHelper(executil.ARGS('git symbolic-ref HEAD'), true, true);
- if (!ref) {
+ var ref;
+ try {
+ ref = yield executil.execHelper(executil.ARGS('git symbolic-ref HEAD'), true, true);
--- End diff --
Fixed this - removed the use of `pretend` as it would not work across control flow anyway.
---
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-coho pull request: Add command to merge PR
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-coho/pull/78#discussion_r30698706
--- Diff: src/main.js ---
@@ -172,11 +177,6 @@ module.exports = function() {
var command;
var argv = optimist
.usage(usage)
- .options('chdir', {
- desc: 'Use --no-chdir to run in your CWD instead of the parent of cordova-coho/',
- type: 'boolean',
- default: true
- })
--- End diff --
Because I don't care for this option in the merge-pr command.
---
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-coho pull request: Add command to merge PR
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on the pull request:
https://github.com/apache/cordova-coho/pull/78#issuecomment-103956908
You need to specify the remote of the branch which is 'target'/'destination' of the PR i.e. https://github.com/apache/cordova-medic.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 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-coho pull request: Add command to merge PR
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on the pull request:
https://github.com/apache/cordova-coho/pull/78#issuecomment-103939716
LGTM. Small question: without `pretend`, could we still do something like a "dry run" of the command before actually running 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 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-coho pull request: Add command to merge PR
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on the pull request:
https://github.com/apache/cordova-coho/pull/78#issuecomment-103947763
Can you share output of git remote -v for this repo? It does not look like `dev` points to the correct github repo. It should point to https://github.com/apache/cordova-medic.git in this case.
---
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-coho pull request: Add command to merge PR
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-coho/pull/78#discussion_r30577291
--- Diff: src/gitutil.js ---
@@ -43,12 +43,14 @@ exports.tagExists = function*(tagName) {
}
exports.retrieveCurrentBranchName = function*(allowDetached) {
- var ref = yield executil.execHelper(executil.ARGS('git symbolic-ref HEAD'), true, true);
- if (!ref) {
+ var ref;
+ try {
+ ref = yield executil.execHelper(executil.ARGS('git symbolic-ref HEAD'), true, true);
--- End diff --
Why is this a call to `execHelper` instead of `execOrPretend`?
---
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-coho pull request: Add command to merge PR
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-coho/pull/78#discussion_r30576714
--- Diff: src/main.js ---
@@ -172,11 +177,6 @@ module.exports = function() {
var command;
var argv = optimist
.usage(usage)
- .options('chdir', {
- desc: 'Use --no-chdir to run in your CWD instead of the parent of cordova-coho/',
- type: 'boolean',
- default: true
- })
--- End diff --
Why is this option removed?
---
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-coho pull request: Add command to merge PR
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-coho/pull/78#discussion_r30577435
--- Diff: src/merge-pr.js ---
@@ -0,0 +1,62 @@
+
+var flagutil = require('./flagutil');
+var optimist = require('optimist');
+var shelljs = require('shelljs');
+var executil = require('./executil');
+var gitutil = require('./gitutil');
+var superspawn = require('./superspawn');
+
+module.exports = function *(argv) {
+ var opt = flagutil.registerHelpFlag(optimist);
+ opt.options('pr', {
+ desc: 'PR # that needs to be merged',
+ demand: true
+ }).options('remote', {
+ desc: 'Named remote for the github apache mirror',
+ demand: true
+ }).options('pretend', {
+ desc: 'Don\'t actually run commands, just print what would be run.',
+ type:'boolean'
+ });
+ argv = opt
+ .usage('Merges the pull request to master\n' +
+ '\n' +
+ 'Usage: $0 merge-pr --pr 111 --remote mirror')
+ .argv;
+ if (argv.h) {
+ optimist.showHelp();
+ process.exit(1);
+ }
+ yield gitutil.stashAndPop("", function*() {
+ yield executil.execOrPretend(executil.ARGS('git checkout master'), argv.pretend);
+
+ yield executil.execOrPretend(executil.ARGS('git pull origin master'), argv.pretend);
+
+ yield executil.execOrPretend(['git', 'fetch', '-fu', argv.remote,
--- End diff --
Just for clarity, please comment what the long versions of `-f` and `-u` are.
---
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-coho pull request: Add command to merge PR
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh closed the pull request at:
https://github.com/apache/cordova-coho/pull/78
---
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-coho pull request: Add command to merge PR
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-coho/pull/78#discussion_r30577302
--- Diff: src/repo-update.js ---
@@ -129,8 +129,9 @@ function *updateRepos(repos, branches, noFetch) {
yield gitutil.gitCheckout(branchName);
var ret = yield executil.execHelper(executil.ARGS('git merge --ff-only', repo.remoteName + '/' + branchName), false, true);
if (ret === null) {
- ret = yield executil.execHelper(executil.ARGS('git rebase ' + repo.remoteName + '/' + branchName), false, true);
- if (ret === null) {
+ try {
+ ret = yield executil.execHelper(executil.ARGS('git rebase ' + repo.remoteName + '/' + branchName), false, true);
--- End diff --
Why is this a call to `execHelper` instead of `execOrPretend`?
---
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-coho pull request: Add command to merge PR
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-coho/pull/78#discussion_r30698950
--- Diff: src/merge-pr.js ---
@@ -0,0 +1,62 @@
+
+var flagutil = require('./flagutil');
+var optimist = require('optimist');
+var shelljs = require('shelljs');
+var executil = require('./executil');
+var gitutil = require('./gitutil');
+var superspawn = require('./superspawn');
+
+module.exports = function *(argv) {
+ var opt = flagutil.registerHelpFlag(optimist);
+ opt.options('pr', {
+ desc: 'PR # that needs to be merged',
+ demand: true
+ }).options('remote', {
+ desc: 'Named remote for the github apache mirror',
+ demand: true
+ }).options('pretend', {
+ desc: 'Don\'t actually run commands, just print what would be run.',
+ type:'boolean'
+ });
+ argv = opt
+ .usage('Merges the pull request to master\n' +
+ '\n' +
+ 'Usage: $0 merge-pr --pr 111 --remote mirror')
+ .argv;
+ if (argv.h) {
+ optimist.showHelp();
+ process.exit(1);
+ }
+ yield gitutil.stashAndPop("", function*() {
+ yield executil.execOrPretend(executil.ARGS('git checkout master'), argv.pretend);
+
+ yield executil.execOrPretend(executil.ARGS('git pull origin master'), argv.pretend);
+
+ yield executil.execOrPretend(['git', 'fetch', '-fu', argv.remote,
+ 'refs/pull/' + argv.pr + '/head:pr/' + argv.pr], argv.pretend);
+
+ try {
+ yield executil.execHelper(executil.ARGS('git merge --ff-only pr/' + argv.pr),
--- End diff --
Done
---
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-coho pull request: Add command to merge PR
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-coho/pull/78#discussion_r30698977
--- Diff: src/merge-pr.js ---
@@ -0,0 +1,62 @@
+
--- End diff --
Good catch! 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 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-coho pull request: Add command to merge PR
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-coho/pull/78#discussion_r30699407
--- Diff: src/merge-pr.js ---
@@ -0,0 +1,62 @@
+
+var flagutil = require('./flagutil');
+var optimist = require('optimist');
+var shelljs = require('shelljs');
+var executil = require('./executil');
+var gitutil = require('./gitutil');
+var superspawn = require('./superspawn');
+
+module.exports = function *(argv) {
+ var opt = flagutil.registerHelpFlag(optimist);
+ opt.options('pr', {
+ desc: 'PR # that needs to be merged',
+ demand: true
+ }).options('remote', {
+ desc: 'Named remote for the github apache mirror',
+ demand: true
+ }).options('pretend', {
+ desc: 'Don\'t actually run commands, just print what would be run.',
+ type:'boolean'
+ });
+ argv = opt
+ .usage('Merges the pull request to master\n' +
+ '\n' +
+ 'Usage: $0 merge-pr --pr 111 --remote mirror')
+ .argv;
+ if (argv.h) {
+ optimist.showHelp();
+ process.exit(1);
+ }
+ yield gitutil.stashAndPop("", function*() {
+ yield executil.execOrPretend(executil.ARGS('git checkout master'), argv.pretend);
+
+ yield executil.execOrPretend(executil.ARGS('git pull origin master'), argv.pretend);
+
+ yield executil.execOrPretend(['git', 'fetch', '-fu', argv.remote,
+ 'refs/pull/' + argv.pr + '/head:pr/' + argv.pr], argv.pretend);
+
+ try {
+ yield executil.execHelper(executil.ARGS('git merge --ff-only pr/' + argv.pr),
+ /*silent*/ true, /*allowError*/ true, argv.pretend);
+ } catch (e) {
+ if (e.message.indexOf('fatal: Not possible to fast-forward, aborting.') > 0) {
+ // Let's try to rebase
+ yield executil.execOrPretend(executil.ARGS('git checkout pr/' + argv.pr), argv.pretend);
+
+ yield executil.execOrPretend(executil.ARGS('git pull --rebase origin master'), argv.pretend);
+
+ yield executil.execOrPretend(executil.ARGS('git checkout master'), argv.pretend);
+
+ yield executil.execOrPretend(executil.ARGS('git merge --ff-only pr/' + argv.pr), argv.pretend);
+
+ var commitMessage = yield executil.execOrPretend(executil.ARGS('git log --format=%B -n 1 HEAD'),
+ argv.pretend);
+
+ yield executil.execOrPretend(['git', 'commit', '--amend', '-m',
--- End diff --
I expect the committer to either ask the author to split commits to a meaningful set or do an interactive rebase after `merge-pr`. I have logged a line at the end requesting the committer to do so if needed.
---
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-coho pull request: Add command to merge PR
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on a diff in the pull request:
https://github.com/apache/cordova-coho/pull/78#discussion_r30577315
--- Diff: src/executil.js ---
@@ -46,13 +47,16 @@ function execHelper(cmdAndArgs, silent, allowError) {
gitCommitCount++;
}
cmdAndArgs[0] = cmdAndArgs[0].replace(/^git /, 'git -c color.ui=always ');
- if (!silent || silent === 3) {
+ if (!silent || silent === 3 || pretend) {
print('Executing:', cmdAndArgs.join(' '));
}
+ if(pretend) {
+ return Q();
+ }
--- End diff --
Isn't it expected that this function only gets called when `pretend` is false?
---
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-coho pull request: Add command to merge PR
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-coho/pull/78#discussion_r30699028
--- Diff: src/merge-pr.js ---
@@ -0,0 +1,62 @@
+
+var flagutil = require('./flagutil');
+var optimist = require('optimist');
+var shelljs = require('shelljs');
+var executil = require('./executil');
+var gitutil = require('./gitutil');
+var superspawn = require('./superspawn');
+
+module.exports = function *(argv) {
+ var opt = flagutil.registerHelpFlag(optimist);
+ opt.options('pr', {
+ desc: 'PR # that needs to be merged',
+ demand: true
+ }).options('remote', {
--- End diff --
I know that would be nice - might get to it in the next iteration. Out of time for this one now.
---
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-coho pull request: Add command to merge PR
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on the pull request:
https://github.com/apache/cordova-coho/pull/78#issuecomment-103941841
There is no 'dry run' feature now. I can see it would nice to preview what the black box command will do - but I don't have a good way to do that especially because of dependencies on output of commands and control flow.
---
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-coho pull request: Add command to merge PR
Posted by agrieve <gi...@git.apache.org>.
Github user agrieve commented on a diff in the pull request:
https://github.com/apache/cordova-coho/pull/78#discussion_r30661140
--- Diff: src/merge-pr.js ---
@@ -0,0 +1,62 @@
+
--- End diff --
missing header
---
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-coho pull request: Add command to merge PR
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on the pull request:
https://github.com/apache/cordova-coho/pull/78#issuecomment-103952930
Command:
git remote -v
Output:
dev git@github.com:MSOpenTech/cordova-medic.git (fetch)
dev git@github.com:MSOpenTech/cordova-medic.git (push)
origin https://git-wip-us.apache.org/repos/asf/cordova-medic.git (fetch)
origin https://git-wip-us.apache.org/repos/asf/cordova-medic.git (push)
---
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-coho pull request: Add command to merge PR
Posted by dblotsky <gi...@git.apache.org>.
Github user dblotsky commented on the pull request:
https://github.com/apache/cordova-coho/pull/78#issuecomment-103945755
Sorry, seems there is a bug somewhere. I tried to merge an actual PR with the following command while inside `cordova-medic` and on branch `CB-9011`:
coho merge-pr --pr 54 --remote dev
Below is the output I got:
```
./ ============================= Executing: git checkout master
Switched to branch 'master'
Your branch is up-to-date with 'origin/master'.
./ ============================= Executing: git pull origin master
From https://git-wip-us.apache.org/repos/asf/cordova-medic
* branch master -> FETCH_HEAD
Already up-to-date.
./ ============================= Executing: git fetch -fu dev refs/pull/54/head:pr/54
fatal: Couldn't find remote ref refs/pull/54/head
```
---
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-coho pull request: Add command to merge PR
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-coho/pull/78#discussion_r30698893
--- Diff: src/executil.js ---
@@ -46,13 +47,16 @@ function execHelper(cmdAndArgs, silent, allowError) {
gitCommitCount++;
}
cmdAndArgs[0] = cmdAndArgs[0].replace(/^git /, 'git -c color.ui=always ');
- if (!silent || silent === 3) {
+ if (!silent || silent === 3 || pretend) {
print('Executing:', cmdAndArgs.join(' '));
}
+ if(pretend) {
+ return Q();
+ }
--- End diff --
Reverted most of these 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 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