From 3b41b1839b36b1063674af6acc659e43a97c8c39 Mon Sep 17 00:00:00 2001 From: Zevs Date: Sun, 3 Jun 2012 21:29:20 +0300 Subject: [PATCH 1/3] ability to remove source branch after merge --- app/controllers/merge_requests_controller.rb | 1 + app/models/merge_request.rb | 3 ++- app/views/merge_requests/show.html.haml | 12 ++++++++---- lib/gitlab/merge.rb | 8 +++++++- 4 files changed, 18 insertions(+), 6 deletions(-) diff --git a/app/controllers/merge_requests_controller.rb b/app/controllers/merge_requests_controller.rb index 7a803e2d..c4bbc747 100644 --- a/app/controllers/merge_requests_controller.rb +++ b/app/controllers/merge_requests_controller.rb @@ -109,6 +109,7 @@ class MergeRequestsController < ApplicationController def automerge return access_denied! unless can?(current_user, :accept_mr, @project) if @merge_request.open? && @merge_request.can_be_merged? + @merge_request.should_remove_source_branch = params[:should_remove_source_branch] @merge_request.automerge!(current_user) @status = true else diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index ded12601..fed761a6 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -14,7 +14,8 @@ class MergeRequest < ActiveRecord::Base serialize :st_diffs attr_protected :author, :author_id, :project, :project_id - attr_accessor :author_id_of_changes + attr_accessor :author_id_of_changes, + :should_remove_source_branch validates_presence_of :project_id validates_presence_of :assignee_id diff --git a/app/views/merge_requests/show.html.haml b/app/views/merge_requests/show.html.haml index 87932807..9a804ee2 100644 --- a/app/views/merge_requests/show.html.haml +++ b/app/views/merge_requests/show.html.haml @@ -62,9 +62,13 @@ .automerge_widget.can_be_merged{:style => "display:none"} .alert.alert-success %span - = link_to "Accept Merge Request", automerge_project_merge_request_path(@project, @merge_request), :class => "btn small info accept_merge_request", :remote => true -   - You can accept this request automatically. If you still want to do it manually - #{link_to "click here", "#", :class => "how_to_merge_link vlink", :title => "How To Merge"} for instructions + = form_for [:automerge, @project, @merge_request], :remote => true, :method => :get do |f| + You can accept this request automatically. If you still want to do it manually - #{link_to "click here", "#", :class => "how_to_merge_link vlink", :title => "How To Merge"} for instructions + %br + = check_box_tag :should_remove_source_branch + = label_tag :should_remove_source_branch, "Remove source-branch" + + = f.submit "Accept Merge Request", :class => "btn small info accept_merge_request" .automerge_widget.cannot_be_merged{:style => "display:none"} .alert.alert-info @@ -108,7 +112,7 @@ current_state: "#{@merge_request.human_state}" }); - $(".accept_merge_request").live("ajax:beforeSend", function() { + $(".edit_merge_request").live("ajax:beforeSend", function() { $(this).replaceWith('#{image_tag "ajax_loader.gif"}'); }) }) diff --git a/lib/gitlab/merge.rb b/lib/gitlab/merge.rb index 695e41e5..74ad70b5 100644 --- a/lib/gitlab/merge.rb +++ b/lib/gitlab/merge.rb @@ -37,7 +37,7 @@ module Gitlab unless project.satellite.exists? raise "You should run: rake gitlab:app:enable_automerge" end - + project.satellite.clear Dir.chdir(project.satellite.path) do @@ -48,6 +48,12 @@ module Gitlab merge_repo.git.sh "git config user.email \"#{user.email}\"" merge_repo.git.sh "git checkout -b #{merge_request.target_branch} origin/#{merge_request.target_branch}" output = merge_repo.git.pull({}, "--no-ff", "origin", merge_request.source_branch) + + #remove source-branch + if merge_request.should_remove_source_branch + merge_repo.git.sh "git push origin :#{merge_request.source_branch}" + end + yield(merge_repo, output) end end From d6ed9920d68691472fe9f9a9247508573c64bae4 Mon Sep 17 00:00:00 2001 From: Zevs Date: Tue, 5 Jun 2012 00:08:41 +0300 Subject: [PATCH 2/3] #888 prevent to remove source_branch --- app/models/merge_request.rb | 2 +- app/models/project/repository_trait.rb | 4 ++++ app/views/merge_requests/show.html.haml | 5 +++-- lib/gitlab/merge.rb | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index fed761a6..2aba24bc 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -189,7 +189,7 @@ class MergeRequest < ActiveRecord::Base self.merge!(current_user.id) true end - rescue + rescue self.mark_as_unmergable false end diff --git a/app/models/project/repository_trait.rb b/app/models/project/repository_trait.rb index a759ead5..2b9cf437 100644 --- a/app/models/project/repository_trait.rb +++ b/app/models/project/repository_trait.rb @@ -114,5 +114,9 @@ module Project::RepositoryTrait def root_ref default_branch || "master" end + + def root_ref? branch + root_ref == branch + end end end diff --git a/app/views/merge_requests/show.html.haml b/app/views/merge_requests/show.html.haml index 9a804ee2..cfa74fc0 100644 --- a/app/views/merge_requests/show.html.haml +++ b/app/views/merge_requests/show.html.haml @@ -65,8 +65,9 @@ = form_for [:automerge, @project, @merge_request], :remote => true, :method => :get do |f| You can accept this request automatically. If you still want to do it manually - #{link_to "click here", "#", :class => "how_to_merge_link vlink", :title => "How To Merge"} for instructions %br - = check_box_tag :should_remove_source_branch - = label_tag :should_remove_source_branch, "Remove source-branch" + -unless @project.root_ref? @merge_request.source_branch + = check_box_tag :should_remove_source_branch + = label_tag :should_remove_source_branch, "Remove source-branch" = f.submit "Accept Merge Request", :class => "btn small info accept_merge_request" diff --git a/lib/gitlab/merge.rb b/lib/gitlab/merge.rb index 74ad70b5..134695ce 100644 --- a/lib/gitlab/merge.rb +++ b/lib/gitlab/merge.rb @@ -50,7 +50,7 @@ module Gitlab output = merge_repo.git.pull({}, "--no-ff", "origin", merge_request.source_branch) #remove source-branch - if merge_request.should_remove_source_branch + if merge_request.should_remove_source_branch && !project.root_ref?(merge_request.source_branch) merge_repo.git.sh "git push origin :#{merge_request.source_branch}" end From 2d68e7f4cd0ce60e340891fd2dfa2e479bb173ec Mon Sep 17 00:00:00 2001 From: randx Date: Tue, 5 Jun 2012 19:45:18 +0300 Subject: [PATCH 3/3] MR -> remove source branch -> UI pollished --- app/assets/stylesheets/common.scss | 48 +++++++++++++++++--- app/assets/stylesheets/gitlab_bootstrap.scss | 12 +++++ app/views/merge_requests/show.html.haml | 21 ++++++--- 3 files changed, 67 insertions(+), 14 deletions(-) diff --git a/app/assets/stylesheets/common.scss b/app/assets/stylesheets/common.scss index b4f7e1e4..57d21360 100644 --- a/app/assets/stylesheets/common.scss +++ b/app/assets/stylesheets/common.scss @@ -714,6 +714,19 @@ p.time { text-align:center; float:left; margin-right:20px; + + &.success { + background: #5BB75B; + color: white; + text-shadow: 0 1px #111; + border-color: #9A9; + } + &.error { + background: #DA4E49; + border-color: #BD362F; + color: white; + text-shadow: 0 1px #111; + } } .arrow{ @@ -864,15 +877,36 @@ li.note { background: #DFF0D8; } - .accept_merge_request { - color: #fff; - text-shadow: 0 1px 1px #222; - background: #5bb75b;; - &:hover { - background-color: #51a351; - color: #fff; + + form { + margin-bottom:0; + .clearfix { + margin-bottom:0; } } + + .accept_group { + float:left; + border: 1px solid #ADA; + padding: 2px; + @include border-radius(5px); + border-radius: 5px; + background: #CEB; + + .accept_merge_request { + float:left; + } + .remove_branch_holder { + margin-left:20px; + margin-right:10px; + float:left; + } + label { + color:#444; + } + } + + .how_to_merge_link { @extend .primary; } diff --git a/app/assets/stylesheets/gitlab_bootstrap.scss b/app/assets/stylesheets/gitlab_bootstrap.scss index b1594d8b..a8a38290 100644 --- a/app/assets/stylesheets/gitlab_bootstrap.scss +++ b/app/assets/stylesheets/gitlab_bootstrap.scss @@ -127,6 +127,18 @@ table { @extend .btn-primary; } + &.success { + color: #fff; + text-shadow: 0 0 1px #111; + background: #5bb75b;; + font-weight: bold; + + &:hover { + background-color: #51a351; + color: #fff; + } + } + &.danger, &.btn-danger { color:#fff; diff --git a/app/views/merge_requests/show.html.haml b/app/views/merge_requests/show.html.haml index cfa74fc0..7707b79b 100644 --- a/app/views/merge_requests/show.html.haml +++ b/app/views/merge_requests/show.html.haml @@ -8,7 +8,7 @@ %span.right - if can?(current_user, :modify_merge_request, @merge_request) - if @merge_request.open? - = link_to 'Close', project_merge_request_path(@project, @merge_request, :merge_request => {:closed => true }, :status_only => true), :method => :put, :class => "btn small padded", :title => "Close merge request" + = link_to 'Close', project_merge_request_path(@project, @merge_request, :merge_request => {:closed => true }, :status_only => true), :method => :put, :class => "btn small padded danger", :title => "Close merge request" = link_to edit_project_merge_request_path(@project, @merge_request), :class => "btn small padded" do %i.icon-edit Edit @@ -63,13 +63,20 @@ .alert.alert-success %span = form_for [:automerge, @project, @merge_request], :remote => true, :method => :get do |f| - You can accept this request automatically. If you still want to do it manually - #{link_to "click here", "#", :class => "how_to_merge_link vlink", :title => "How To Merge"} for instructions - %br - -unless @project.root_ref? @merge_request.source_branch - = check_box_tag :should_remove_source_branch - = label_tag :should_remove_source_branch, "Remove source-branch" + %p + You can accept this request automatically. + If you still want to do it manually - + %strong= link_to "click here", "#", :class => "how_to_merge_link vlink", :title => "How To Merge" + for instructions + .accept_group + = f.submit "Accept Merge Request", :class => "btn small success accept_merge_request" + - unless @project.root_ref? @merge_request.source_branch + .remove_branch_holder + = label_tag :should_remove_source_branch, :class => "checkbox" do + = check_box_tag :should_remove_source_branch + Remove source-branch + .clearfix - = f.submit "Accept Merge Request", :class => "btn small info accept_merge_request" .automerge_widget.cannot_be_merged{:style => "display:none"} .alert.alert-info