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_branchparameter 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 isfalse- the name intentionally doesn't follow the current naming convention used in the merge_request model, namely, either using
forceorshouldprefix, similarly toforce_remove_source_branchandshould_remove_source_branch. Reason: the currentforce...andshould...attributes should be rather generalized, especially as none of these attribute names refer their actual effect IMHO (for me,forcerather behaves asshould, 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
downmethod so the migration can be reverted - Added the output of the migration(s) to the MR body
- Added tests for the migration in
spec/migrationsif necessary (e.g. when migrating data)- Author's comment: didn't seem necessary due to the simplicity of the migration
General checklist
- Changelog entry added, if necessary
- Documentation created/updated
- Tests added for this feature/bug
- API support
- port to EE
- gitlab-ee!14252
What are the relevant issue numbers?
Closes #32884 (moved), gitlab#18283 (closed)
Edited by James Ramsay (ex-GitLab)