Remove merge_when_checks_pass feature flag

changed milestone to %17.7

added Category:Security Policy Management Deliverable Technical Writing backend customer devopscreate documentation feature flag featureenhancement groupcode review missed-deliverable missed:17.0 missed:17.1 missed:17.3 planningto-do sectiondev twtriaged typefeature labels

assigned to @marc_shaw

added 588 commits

4fe6cd83...72904314 - 587 commits from branch master
1d8cdf80 - Remove merge_when_checks_pass feature flag

Compare with previous version

added frontend label

	4 Warnings
	This merge request is quite big (718 lines changed), please consider splitting it into multiple merge requests.
	a608af1c: The commit subject must contain at least 3 words. For more information, take a look at our Commit message guidelines.
	3a510955: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines.
	404693ed: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines.

	1 Message
	This merge request adds or changes documentation files. A review from the Technical Writing team before you merge is recommended. Reviews can happen after you merge.

Documentation review

The following files require a review from a technical writer:

doc/user/project/merge_requests/auto_merge.md (Link to current live version)

The review does not need to block merging this merge request. See the:

Metadata for the *.md files that you've changed. The first few lines of each *.md file identify the stage and group most closely associated with your docs change.
The Technical Writer assigned for that stage and group.
Documentation workflows for information on when to assign a merge request for review.

Reviewer roulette

Category	Reviewer	Maintainer
backend	`@zmartins` (UTC+1, 6 hours behind author)	`@michold` (UTC+1, 6 hours behind author)
test for `spec/features/*`	`@zmartins` (UTC+1, 6 hours behind author)	Maintainer review is optional for test for `spec/features/*`

Please refer to documentation page for guidance on how you can benefit from the Reviewer Roulette, or use the GitLab Review Workload Dashboard to find other available reviewers.

If needed, you can retry the danger-review job that generated this comment.

Generated by Danger

Bundle size analysis [beta]

This compares changes in bundle size for entry points between the commits 7dbb987b and 24f44d29

Special assets

Entrypoint / Name	Size before	Size after	Diff	Diff in percent
average	4.39 MB	4.39 MB	-	-0.0 %
mainChunk	3.31 MB	3.31 MB	-	0.0 %

Note: We do not have exact data for 7dbb987b. So we have used data from: f2166d65.
The intended commit has no webpack pipeline, so we chose the last commit with one before it.

Please look at the full report for more details

Read more about how this report works.

Generated by Danger

added 1 commit

9410b383 - Remove merge_when_checks_pass feature flag

Compare with previous version

added 1 commit

abe0e20a - Remove merge_when_checks_pass feature flag

Compare with previous version

added 1 commit

6aab28b3 - Remove merge_when_checks_pass feature flag

Compare with previous version

added 1 commit

1094b543 - Remove merge_when_checks_pass feature flag

Compare with previous version

added 1 commit

24f44d29 - Remove merge_when_checks_pass feature flag

Compare with previous version

marked this merge request as ready

changed title from {-Draft: Resolve "[Feature flag] Rollout of -}merge{-_-}when{-_checks_pass"-} to Remove merge when pipeline succeeds services

changed the description

added 1 commit

23808db1 - Only remove the feature flag and keep the old service

Compare with previous version

changed title from Remove merge when pipeline succeeds services to Remove merge_when_checks_pass feature flag

changed the description

added 1 commit

2dfff297 - Only remove the feature flag and keep the old service

Compare with previous version

added 718 commits

2dfff297...14290667 - 716 commits from branch master
5f517cc0 - Remove merge_when_checks_pass feature flag
6f9956ca - Only remove the feature flag and keep the old service

Compare with previous version

resolved all threads

Hola @patrickbajao

Mind taking the first look here ?

requested review from @jacopo-beschi

added 1 commit

e4f03a2e - Only remove the feature flag and keep the old service

Compare with previous version

requested review from @patrickbajao and removed review request for @jacopo-beschi

Generated by gitlab_quality-test_tooling.

Slow tests detected in this merge request. These slow tests might be related to this merge request's changes.

Click to expand

Job	File	Name	Duration	Expected duration
#8269071258	`spec/features/merge_request/user_sees_merge_widget_spec.rb#L38`	Merge request > User sees merge widget new merge request shows widget status after creating new merge request	53.59 s	< 50.13 s
#8292208305	`spec/features/merge_request/user_resolves_wip_mr_spec.rb#L41`	Merge request > User resolves Draft when there is active pipeline for merge request retains merge request data after clicking Resolve WIP status	52.02 s	< 50.13 s
#8299219395	`spec/features/discussion_comments/merge_request_spec.rb#L20`	Thread Comments Merge Request behaves like thread comments for issue, epic and merge request clicking "Comment" will post a comment	53.46 s	< 50.13 s
#8320163882	`ee/spec/lib/ee/gitlab/import_export/project/tree_restorer_spec.rb#L119`	Gitlab::ImportExport::Project::TreeRestorer restores `protected_environments` with `deploy_access_levels` is expected to eq 1	29.38 s	< 27.12 s
#8403259539	`spec/features/merge_request/user_sees_merge_widget_spec.rb#L38`	Merge request > User sees merge widget new merge request shows widget status after creating new merge request	52.69 s	< 50.13 s

added rspec:slow test detected label

added 1 commit

6e9d393a - Only remove the feature flag and keep the old service

Compare with previous version

added 1 commit

adff936c - Only remove the feature flag and keep the old service

Compare with previous version

added 2 commits

b619f153 - wip
3dc92ce4 - Allow for existing MWPS pipelines to still run

Compare with previous version

requested review from @patrickbajao

added pipeline:run-all-e2e label

added 1 commit

3aca42a6 - Allow for existing MWPS pipelines to still run

Compare with previous version

added 1 commit

690769ea - Allow for existing MWPS pipelines to still run

Compare with previous version

E2E Test Result Summary

allure-report-publisher generated test report!

e2e-test-on-gdk: test report for a608af1c

expand test summary

+------------------------------------------------------------------+
|                          suites summary                          |
+-------------+--------+--------+---------+-------+-------+--------+
|             | passed | failed | skipped | flaky | total | result |
+-------------+--------+--------+---------+-------+-------+--------+
| Create      | 129    | 0      | 22      | 0     | 151   | ✅     |
| Govern      | 75     | 0      | 3       | 0     | 78    | ✅     |
| Plan        | 76     | 0      | 0       | 0     | 76    | ✅     |
| Package     | 24     | 0      | 11      | 0     | 35    | ✅     |
| Verify      | 43     | 0      | 2       | 0     | 45    | ✅     |
| Secure      | 4      | 0      | 0       | 0     | 4     | ✅     |
| Data Stores | 33     | 0      | 1       | 0     | 34    | ✅     |
| Analytics   | 2      | 0      | 0       | 0     | 2     | ✅     |
| Monitor     | 8      | 0      | 0       | 0     | 8     | ✅     |
| Ai-powered  | 0      | 0      | 1       | 0     | 1     | ➖     |
| Release     | 5      | 0      | 0       | 0     | 5     | ✅     |
| Fulfillment | 2      | 0      | 0       | 0     | 2     | ✅     |
| Manage      | 1      | 0      | 1       | 0     | 2     | ✅     |
+-------------+--------+--------+---------+-------+-------+--------+
| Total       | 402    | 0      | 41      | 0     | 443   | ✅     |
+-------------+--------+--------+---------+-------+-------+--------+

e2e-test-on-cng: test report for a608af1c

expand test summary

+------------------------------------------------------------------+
|                          suites summary                          |
+-------------+--------+--------+---------+-------+-------+--------+
|             | passed | failed | skipped | flaky | total | result |
+-------------+--------+--------+---------+-------+-------+--------+
| Plan        | 82     | 0      | 8       | 0     | 90    | ✅     |
| Govern      | 82     | 0      | 9       | 1     | 91    | ✅     |
| Verify      | 49     | 0      | 16      | 0     | 65    | ✅     |
| Package     | 24     | 0      | 14      | 0     | 38    | ✅     |
| Ai-powered  | 0      | 0      | 2       | 0     | 2     | ➖     |
| Monitor     | 8      | 0      | 12      | 0     | 20    | ✅     |
| Fulfillment | 2      | 0      | 7       | 1     | 9     | ✅     |
| Create      | 136    | 0      | 20      | 0     | 156   | ✅     |
| Data Stores | 33     | 0      | 10      | 0     | 43    | ✅     |
| Configure   | 0      | 0      | 3       | 0     | 3     | ➖     |
| Growth      | 0      | 0      | 2       | 0     | 2     | ➖     |
| Manage      | 1      | 0      | 9       | 0     | 10    | ✅     |
| Release     | 5      | 0      | 1       | 0     | 6     | ✅     |
| Secure      | 2      | 0      | 5       | 0     | 7     | ✅     |
| ModelOps    | 0      | 0      | 1       | 0     | 1     | ➖     |
| Analytics   | 2      | 0      | 0       | 1     | 2     | ✅     |
+-------------+--------+--------+---------+-------+-------+--------+
| Total       | 426    | 0      | 119     | 3     | 545   | ✅     |
+-------------+--------+--------+---------+-------+-------+--------+

e2e-test-on-omnibus: test report for a608af1c

expand test summary

+---------------------------------------------------------------------+
|                           suites summary                            |
+----------------+--------+--------+---------+-------+-------+--------+
|                | passed | failed | skipped | flaky | total | result |
+----------------+--------+--------+---------+-------+-------+--------+
| Create         | 563    | 0      | 81      | 8     | 644   | ✅     |
| Verify         | 49     | 0      | 16      | 0     | 65    | ✅     |
| Plan           | 83     | 0      | 8       | 0     | 91    | ✅     |
| Govern         | 95     | 0      | 8       | 0     | 103   | ✅     |
| Manage         | 26     | 0      | 19      | 1     | 45    | ✅     |
| Package        | 32     | 0      | 13      | 0     | 45    | ✅     |
| Ai-powered     | 1      | 0      | 2       | 1     | 3     | ✅     |
| Monitor        | 12     | 0      | 13      | 0     | 25    | ✅     |
| Data Stores    | 33     | 0      | 10      | 0     | 43    | ✅     |
| Release        | 5      | 0      | 1       | 0     | 6     | ✅     |
| Analytics      | 3      | 0      | 0       | 0     | 3     | ✅     |
| Configure      | 1      | 0      | 3       | 0     | 4     | ✅     |
| Systems        | 6      | 0      | 1       | 0     | 7     | ✅     |
| Secure         | 5      | 0      | 3       | 0     | 8     | ✅     |
| Fulfillment    | 4      | 0      | 7       | 0     | 11    | ✅     |
| GitLab Metrics | 2      | 0      | 1       | 0     | 3     | ✅     |
| ModelOps       | 0      | 0      | 1       | 0     | 1     | ➖     |
| Growth         | 0      | 0      | 2       | 0     | 2     | ➖     |
+----------------+--------+--------+---------+-------+-------+--------+
| Total          | 920    | 0      | 189     | 10    | 1109  | ✅     |
+----------------+--------+--------+---------+-------+-------+--------+

requested review from @patrickbajao

I have reviewed the merge request and left several comments, primarily focusing on the removal of the 'merge_when_checks_pass' feature flag and its implications. I estimate there is a decent amount of work required to address these concerns. The review covers topics such as potential unintended consequences, test coverage, and consistency in code changes related to the feature flag removal.

mentioned in issue #492104 (closed)

requested review from @patrickbajao

added 1 commit

7383145a - Code review feedback

Compare with previous version

approved this merge request

added pipeline:mr-approved label

added pipelinetier-3 pipeline:run-e2e-omnibus-once labels

requested review from @aqualls

requested review from @mfanGitLab

Hola @mfanGitLab

Mind taking a maintainer review here?

removed pipeline:run-e2e-omnibus-once label

approved this merge request

Aside: Would this be typemaintenance ? As this isn't a new FF

@marc_shaw

New documentation or new feature flags that relate to ~"type::feature" are considered ~"type::feature".

Per:

typemaintenance: Upkeeping efforts & catch-up corrective improvements that are not Features nor Bugs. This includes removing or altering feature flags

https://handbook.gitlab.com/handbook/product/groups/product-analysis/engineering/metrics/#work-type-classification

mentioned in merge request !170562 (closed)

I have small text changes, but I'm not blocking this merge request. I need to create a docs follow-up after this MR and Remove merge_when_checks_pass_merge_train featu... (!170562 - closed) both merge. I'll do most of my work there.

approved this merge request

mentioned in issue #503129 (closed)

added maintenancerefactor label

added typemaintenance label and removed featureenhancement typefeature labels

added 1 commit

11f8e7e2 - Docs update

Compare with previous version

added 1032 commits

11f8e7e2...5678bc6e - 1027 commits from branch master
4dafc8fd - Remove merge_when_checks_pass feature flag
404693ed - Only remove the feature flag and keep the old service
3a510955 - Allow for existing MWPS pipelines to still run
c37cbcfb - Code review feedback
a608af1c - Docs update

Compare with previous version

reset approvals from @patrickbajao, @aqualls, and @mfanGitLab by pushing to the branch

approved this merge request

mentioned in merge request !172461 (merged)

I've created Clean out all hints of auto-merge feature flags... (!172461 - merged) to finish up the docs updates. It's dependent on this merge request.

mentioned in issue #412995 (closed)

Assigning to @patrickbajao while I am PTO

We are just waiting for 17.7 to merge.

assigned to @patrickbajao

approved this merge request

It's 17.7 now! Lets get this

resolved all threads

enabled automatic add to merge train when checks pass

@mfanGitLab I don't think the 17.6 release has been tagged yet, so merging this now might still means it gets into 17.6. Can we wait a bit longer here?

cc: @francoisrose