apache / cordova-coho

Apache Cordova coho
Apache License 2.0
33 stars 62 forks source link

Add command to merge PR #78

Closed nikhilkh closed 9 years ago

nikhilkh commented 9 years ago

Merging github PR the 'right' way can require a bunch of commands. This change automates that - given the PR number and github remote name.

agrieve commented 9 years ago

Awesome addition!

nikhilkh commented 9 years ago

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.

agrieve commented 9 years ago

LGTM!

dblotsky commented 9 years ago

LGTM. Small question: without pretend, could we still do something like a "dry run" of the command before actually running it?

nikhilkh commented 9 years ago

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.

dblotsky commented 9 years ago

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
nikhilkh commented 9 years ago

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.

dblotsky commented 9 years ago

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)
nikhilkh commented 9 years ago

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

nikhilkh commented 9 years ago

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.

nikhilkh commented 9 years ago

Merged with e337cab.