Skip to content

Add configurable project option for deleting source branch

What does this MR do?

Adds a new configurable setting to Project General Merge requests settings to enable changing the default value of 'Delete source branch' option.

General behaviour

By enabling this Project setting, the following happens:

  • non-protected source branches in new MRs are offered for deletion by default
  • the default option can be always overridden later in:
    • MR edit view
    • MR accept view (right before merge)
    • MR creation view
    • (also, if the MR is created with a push, the necessary force_remove_source_branch parameter can be passed to override the project setting)
  • protected branches are not affected

Extra BE related info

New Project attribute:

  • remove_source_branch_after_merge, default value is false
  • the name intentionally doesn't follow the current naming convention used in the merge_request model, namely, either using force or should prefix, similarly to force_remove_source_branch and should_remove_source_branch. Reason: the current force... and should... attributes should be rather generalized, especially as none of these attribute names refer their actual effect IMHO (for me, force rather behaves as should, and vice versa). Ref comment

If force_remove_source_branch in MR is given then it always prevails project attribute remove_source_branch_after_merge.

Extra FE related info

Inspired by this comment.

Database migration output

== 20190609164758 AddRemoveSourceBranchAfterMergeToProjects: migrating ========
-- add_column(:projects, :remove_source_branch_after_merge, :boolean)
   -> 0.0174s
== 20190609164758 AddRemoveSourceBranchAfterMergeToProjects: migrated (0.0175s) 

Are there points in the code the reviewer needs to double check?

The project attributes must be repeated in many places. Might worth double checking if I didn't miss anything important.

Database checklist

  • Updated db/schema.rb
  • Added a down method so the migration can be reverted
  • Added the output of the migration(s) to the MR body
  • Added tests for the migration in spec/migrations if necessary (e.g. when migrating data)
    • Author's comment: didn't seem necessary due to the simplicity of the migration

General checklist

What are the relevant issue numbers?

Closes #32884 (moved), gitlab#18283 (closed)

Edited by James Ramsay (ex-GitLab)

Merge request reports