Skip to content

Commit

Permalink
[Issue reenhanced#149] Fix for PR ending as "Closed" instead of "Merg…
Browse files Browse the repository at this point in the history 
…ed" - Calling Github API to Squash Merge the PR as opposed doing it manually
  • Loading branch information
@simonzhu24
simonzhu24 committed May 7, 2016
1 parent 06c74f4 commit 71bdd95
Show file tree
Hide file tree
Showing 13 changed files with 565 additions and 396 deletions.
159 changes: 84 additions & 75 deletions README.rdoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ After making commits to your branch, run +review+. Didn't push it up? We don't c

git reflow review -t <title> -m <message>

If you do not pass the title or message options to the review command, you will be given an editor to write your PR request in, similar to `git commit`. The first line is the title, the rest is the body.
If you do not pass the title or message options to the review command, you will be given an editor to write your PR request commit message, similar to `git commit`. The first line is the title, the rest is the body.

$ git reflow review

Expand Down Expand Up @@ -205,64 +205,52 @@ You kick butt. You've got your code reviewed and now you're ready to merge it do
Reflow's +deliver+ requires you to have a pull request, so you'll be protected on those mornings when the coffee isn't working yet.
We built this <b>to get in your way and make you follow the process</b>. If you don't like it, do it by hand. You already know what you're doing.

You'll be presented with a prefilled commit message based on the body of your pull request which includes the text <code>Closes #XX</code> that will automatically close the associated pull request on github when deliver completes.
You'll be presented with a prefilled commit message based on the body of your pull request with references to the pull request and reviewers.

Make a mistake and deliver too early? It happens. You'll be prompted after you edit your commit message if you want to push your updated +master+ to github. If you answer 'n', then you'll want to do <code>git reset --hard origin/master</code> and checkout your branch again.
Want to clean up your feature branch afterwards? You'll be prompted after you edit your commit message if you want to clean up your +feature_branch+ on github. If you answer 'n', then your feature_branch will exist for you to clean it up later.

This is what it looks like:

$ git reflow deliver
From github.com:reenhanced/gitreflow
* branch master -> FETCH_HEAD
Merging pull request #36: 'Enforce at least one LGTM before delivery', from 'reenhanced:nh-fail-without-lgtm' into 'reenhanced:master'
Here's the status of your review:
branches: simonzhu24:test1234 -> simonzhu24:master
number: 51
reviewed by:
url: https://github.com/simonzhu24/test/pull/51

[notice] No one has reviewed your pull request.
Would you like to open it in your browser? n
This is the current status of your Pull Request. Are you sure you want to deliver? Y
Merging pull request #51: 'last commit message', from 'simonzhu24:test1234' into 'simonzhu24:master'
git checkout master
Switched to branch 'master'
From github.com:reenhanced/gitreflow
Your branch is ahead of 'origin/master' by 1 commit.
(use "git push" to publish your local commits)

[success] Pull Request successfully merged.
Would you like to cleanup your feature branch? Y
git pull origin master
remote: Counting objects: 1, done.
remote: Total 1 (delta 0), reused 0 (delta 0), pack-reused 0
Unpacking objects: 100% (1/1), done.
From https://github.com/simonzhu24/test
* branch master -> FETCH_HEAD
Already up-to-date.
Switched to branch 'nh-fail-without-lgtm'
Switched to branch 'master'
Updating c2ec1b1..f90e111
0d8f5e0..f853efa master -> origin/master
Updating 0b6782f..f853efa
Fast-forward
Squash commit -- not updating HEAD
lib/git_reflow.rb | 71 +++++++++++++++++++++++++++----------------------------
1 file changed, 35 insertions(+), 36 deletions(-)
[master d1b4dd5] Enforces LGTM before deliver, even with no comments.
1 file changed, 35 insertions(+), 36 deletions(-)
Merge complete!
Would you like to push this branch to your remote repo and cleanup your feature branch? y
Counting objects: 7, done.
Delta compression using up to 16 threads.
Compressing objects: 100% (4/4), done.
Writing objects: 100% (4/4), 1.38 KiB, done.
Total 4 (delta 2), reused 0 (delta 0)
To git@github.com:reenhanced/gitreflow.git
c2ec1b1..d1b4dd5 master -> master

To git@github.com:reenhanced/gitreflow.git
- [deleted] nh-fail-without-lgtm

Deleted branch nh-fail-without-lgtm (was f90e111).
README.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
git push origin master
Everything up-to-date

git push origin :test1234
To https://github.com/simonzhu24/test.git
- [deleted] test1234

git branch -D test1234
Deleted branch test1234 (was e130c7a).
Nice job buddy.

This is what the default commit message looks like:
Enforces LGTM before deliver, even with no comments.
Removes the need to review the pull request yourself if you make a
comment.

Better error message if setup fails.

Bug fixes.

Closes #36

LGTM given by: @codenamev

Squashed commit of the following:

commit f90e111
Author: Nicholas Hance <nhance@reenhanced.com>
Date: Thu Jul 11 15:33:29 2013 -0400
...

==== How it works
This is what we do behind the scenes when you do +deliver+

Expand All @@ -274,29 +262,43 @@ This is what we do behind the scenes when you do +deliver+

If not, show a list of authors that need to provide a +LGTM+.

If the review is done, it's time to merge. Here's what happens:
* If the review is done, it's time to merge. Here's what happens:

First, update our local +master+ so we don't get conflicts
git checkout master
git pull origin master
First, we use the github_api gem to merge the pull request.

Next, squash merge our feature branch
git merge --squash nh-branch-name

Now, we'll apply a little magic so we have a great commit message by default. Your editor will open and you'll see a nice default for your commit message based on the pull request body.
We call the public API for:

github.pull_requests.merge 'user-name', 'repo-name', 'number', payload

Once you've saved the commit, prompt the user to see if we should continue
Merge complete!
Would you like to push this branch to your remote repo and cleanup your feature branch?
Please take a look at lib/git_reflow/git_server/git_hub/pull_request.rb:102-107 for an example of the call.
This call makes an HTTP request using the Github API to merge the available pull request passing in the user name, repository name, pull request number, and some additional data in the payload.

The payload contains data in the following format:

data = {
"commit_title",
"commit_message",
"sha",
"squash"
}

Notice: "squash" is set to true, meaning that we will do a squash-merge for each pull request.

For more detailed documentation, please read: https://github.com/piotrmurach/github/blob/master/lib/github_api/client/pull_requests.rb#L197

* Next, we prompt you if you want to cleanup
Would you like cleanup your feature branch?

If 'y', then we'll push to +master+ and delete the feature branch
git push origin master
git push origin :nh-branch-name
git branch -D nh-branch-name
If 'y', then we'll update the +base+ and delete the feature branch

git pull origin #{base_branch}
git push origin #{base_branch}
git push origin :#{feature_branch}
git branch -D #{feature_branch}

If 'n', then just stop here. The user can reset his local +master+.
If 'n', then just stop here. The user can clean up his branch locally.

And we're done.
And we're done.

== Guiding principles:
* Your workflow should resemble the following:
Expand All @@ -321,23 +323,30 @@ http://reenhanced.com/images/reflow.png

* If you make a new commit in your branch, you require another review.

* All participants in a pull request must approve the pull request.
If 2 people comment, you need 2 'LGTM's before the code is ready to merge.
* Depending on the minimumApprovals that you specify in your ~/.gitconfig.reflow (created upon reflow setup), you can have the following:

"" : All participants in a pull request must approve the pull request.
"0": 0 approvals required for you to merge PR.
"1": You need a minimum of 1 LGTM and the last comment on your PR must be an LGTM.
"2": You need a minimum of 2 LGTM and the last comment on your PR must be an LGTM.
...

* Once approved, your feature branch is squash merged to master.
This makes the history of the master branch extremely clean and easy to follow.
* Once approved, your feature branch is squash-merged to your base_branch.
This makes the history of the base_branch branch extremely clean and easy to follow.

* Git blame becomes your friend. You'll know who to blame and can see the full context of changes.
Squash commits to master mean every commit represents the whole feature, not a "typo fix".
Squash commits to base_branch mean every commit represents the whole feature, not a "typo fix".


== Configuration

In order to streamline delivery you can set the following git config to
always push the branch after merge and clean up the feature branch.
In order to streamline delivery you can set the following git config to:

git config --global --add "reflow.always-deploy-and-cleanup" "true"
1. always clean up the feature branch after the PR is merged
2. always deliver without further prompt

git config --global --add "reflow.always-cleanup" "true"
git config --global --add "reflow.always-deliver" "true"

---

Expand Down
104 changes: 52 additions & 52 deletions lib/git_reflow.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
require 'git_reflow/os_detector'
require 'git_reflow/sandbox'
require 'git_reflow/git_helpers'
require 'git_reflow/merge_error'

module GitReflow
include Sandbox
Expand All @@ -35,21 +36,21 @@ def status(destination_branch)
end

def review(options = {})
options['base'] ||= 'master'
options[:base] ||= 'master'
create_pull_request = true

fetch_destination options['base']
fetch_destination options[:base]

begin
push_current_branch

existing_pull_request = git_server.find_open_pull_request( from: current_branch, to: options['base'] )
existing_pull_request = git_server.find_open_pull_request( from: current_branch, to: options[:base] )
if existing_pull_request
say "A pull request already exists for these branches:", :notice
existing_pull_request.display_pull_request_summary
ask_to_open_in_browser(existing_pull_request.html_url)
else
unless options['title'] || options['body']
unless options[:title] || options[:body]
pull_request_msg_file = "#{GitReflow.git_root_dir}/.git/GIT_REFLOW_PR_MSG"
default_editor = "#{ENV['EDITOR']}"

Expand All @@ -58,7 +59,7 @@ def review(options = {})
end

File.open(pull_request_msg_file, 'w') do |file|
file.write(options['title'] || GitReflow.current_branch)
file.write(options[:title] || GitReflow.current_branch)
end

GitReflow.run("#{default_editor} #{pull_request_msg_file}", with_system: true)
Expand All @@ -72,23 +73,23 @@ def review(options = {})
pr_msg.shift if pr_msg.first.empty?
end

options['title'] = title
options['body'] = "#{pr_msg.join("\n")}\n"
options[:title] = title
options[:body] = "#{pr_msg.join("\n")}\n"

puts "\nReview your PR:\n"
puts "--------\n"
puts "Title:\n#{options['title']}\n\n"
puts "Body:\n#{options['body']}\n"
puts "Title:\n#{options[:title]}\n\n"
puts "Body:\n#{options[:body]}\n"
puts "--------\n"

create_pull_request = ask("Submit pull request? (Y)") =~ /y/i
end

if create_pull_request
pull_request = git_server.create_pull_request(title: options['title'] || options['body'],
body: options['body'],
pull_request = git_server.create_pull_request(title: options[:title] || options[:body],
body: options[:body],
head: "#{remote_user}:#{current_branch}",
base: options['base'])
base: options[:base])

puts "Successfully created pull request ##{pull_request.number}: #{pull_request.title}\nPull Request URL: #{pull_request.html_url}\n"
ask_to_open_in_browser(pull_request.html_url)
Expand All @@ -103,13 +104,17 @@ def review(options = {})
end
end

def cleanup_feature_branch?
GitReflow::Config.get('reflow.always-cleanup') == "true" || (ask "Would you like to cleanup your feature branch? ") =~ /^y/i
end

def deliver?
GitReflow::Config.get('reflow.always-deliver') == "true" || (ask "This is the current status of your Pull Request. Are you sure you want to deliver? ") =~ /^y/i
end

def deliver(options = {})
feature_branch = current_branch
base_branch = options['base'] || 'master'

update_current_branch
fetch_destination(base_branch)
update_destination(feature_branch)
base_branch = options[:base] || 'master'

begin
existing_pull_request = git_server.find_open_pull_request( :from => current_branch, :to => base_branch )
Expand All @@ -118,42 +123,37 @@ def deliver(options = {})
say "No pull request exists for #{remote_user}:#{current_branch}\nPlease submit your branch for review first with \`git reflow review\`", :deliver_halted
else

commit_message = if "#{existing_pull_request.description}".length > 0
existing_pull_request.description
else
"#{get_first_commit_message}"
end

if existing_pull_request.good_to_merge?(force: options['skip_lgtm'])
puts "Merging pull request ##{existing_pull_request.number}: '#{existing_pull_request.title}', from '#{existing_pull_request.feature_branch_name}' into '#{existing_pull_request.base_branch_name}'"

update_destination(base_branch)
merge_feature_branch(feature_branch,
:destination_branch => base_branch,
:pull_request_number => existing_pull_request.number,
:lgtm_authors => existing_pull_request.approvals,
:message => commit_message)
committed = run_command_with_label 'git commit', with_system: true

if committed
say "Merge complete!", :success

# check if user always wants to push and cleanup, otherwise ask
always_deploy_and_cleanup = GitReflow::Config.get('reflow.always-deploy-and-cleanup') == "true"
deploy_and_cleanup = always_deploy_and_cleanup || (ask "Would you like to push this branch to your remote repo and cleanup your feature branch? ") =~ /^y/i

if deploy_and_cleanup
run_command_with_label "git push origin #{base_branch}"
run_command_with_label "git push origin :#{feature_branch}"
run_command_with_label "git branch -D #{feature_branch}"
puts "Nice job buddy."
else
puts "Cleanup halted. Local changes were not pushed to remote repo.".colorize(:red)
puts "To reset and go back to your branch run \`git reset --hard origin/#{base_branch} && git checkout #{feature_branch}\`"
end
if existing_pull_request.good_to_merge?(force: options[:skip_lgtm])
# displays current status and prompts user for confirmation
self.status base_branch

if self.deliver?
begin
committed = existing_pull_request.merge!(options)

if committed
say "Pull Request successfully merged.", :success

# Pulls merged changes from remote base_branch
run_command_with_label "git pull origin #{base_branch}"

if self.cleanup_feature_branch?
run_command_with_label "git push origin :#{feature_branch}"
run_command_with_label "git branch -D #{feature_branch}"
puts "Nice job buddy."
else
puts "Cleanup halted. Local changes were not pushed to remote repo.".colorize(:red)
puts "To reset and go back to your branch run \`git reset --hard origin/#{base_branch} && git checkout #{feature_branch}\`"
end
else
say "There were problems commiting your feature... please check the errors above and try again.", :error
end
rescue GitReflow::GitServer::MergeError => error
say error.to_s, :deliver_halted
end
else
say "There were problems commiting your feature... please check the errors above and try again.", :error
end
say "Merge aborted", :deliver_halted
end
else
say existing_pull_request.rejection_message, :deliver_halted
end
Expand Down
8 changes: 4 additions & 4 deletions lib/git_reflow/commands/deliver.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@
c.arg_name 'base_branch - the branch you want to merge into'
c.switch [:f, :'skip-lgtm'], desc: 'skip the lgtm checks and deliver your feature branch'
c.action do |global_options,options,args|
deliver_options = {'base' => nil, 'head' => nil, 'skip_lgtm' => options[:'skip-lgtm']}
deliver_options = {:base => nil, :head => nil, :skip_lgtm => options[:'skip-lgtm']}
case args.length
when 2
deliver_options['base'] = args[0]
deliver_options['head'] = args[1]
deliver_options[:base] = args[0]
deliver_options[:head] = args[1]
when 1
deliver_options['base'] = args[0]
deliver_options[:base] = args[0]
end
GitReflow.deliver deliver_options
end
Expand Down
Loading

0 comments on commit 71bdd95

Please sign in to comment.