Only make Review App deployment automatic for frontend changes
What does this MR do?
This makes Review App deployment and smoke end-to-end run against them automatic for frontend-related changes, and manual otherwise.
For non-frontend changes, people should just have to start the review-build-cng
job and the job would automatically play the review-deploy
job once finished (e.g. https://gitlab.com/gitlab-org/gitlab/-/jobs/477627203), and the review-deploy
job would do the same and automatically play the review-qa-smoke
and review-performance
jobs once finished.
Does this MR meet the acceptance criteria?
Conformity
-
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Separation of EE specific content
Closes #208834 (closed).
Merge request reports
Activity
changed milestone to %12.9
added Engineering Productivity workflowplanning breakdown + 1 deleted label
1 Warning 44fa3382: The commit subject length is acceptable, but please try to reduce it to 50 characters. Commit message standards
One or more commit messages do not meet our Git commit message standards.
For more information on how to write a good commit message, take a look at How to Write a Git Commit Message.
Here is an example of a good commit message:
Reject ruby interpolation in externalized strings When using ruby interpolation in externalized strings, they can't be detected. Which means they will never be presented to be translated. To mix variables into translations we need to use `sprintf` instead. Instead of: _("Hello #{subject}") Use: _("Hello %{subject}") % { subject: 'world' }
This is an example of a bad commit message:
updated README.md
This commit message is bad because although it tells us that README.md is updated, it doesn't tell us why or how it was updated.
Reviewer roulette
Changes that require review have been detected! A merge request is normally reviewed by both a reviewer and a maintainer in its primary category (e.g. frontend or backend), and by a maintainer in all other categories.
To spread load more evenly across eligible reviewers, Danger has randomly picked a candidate for each review slot. Feel free to override this selection if you think someone else would be better-suited, or the chosen person is unavailable.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines.
Once you've decided who will review this merge request, mention them as you normally would! Danger does not (yet?) automatically notify them for you.
Category Reviewer Maintainer Engineering Productivity for CI, Danger Albert Salim ( @caalberts
)No maintainer available Generated by
DangerEdited by 🤖 GitLab Bot 🤖added 1 commit
- 277c89db - Only make Review App deployment automatic for frontend changes
added 1 commit
- 2c71c315 - Only make Review App deployment automatic for frontend changes
mentioned in issue #199979 (closed)
We're currently facing #199979 (closed) which makes the pipeline stay in the running state indefinitely...
added workflowblocked label and removed workflowplanning breakdown label
mentioned in issue #208834 (closed)
changed milestone to %Backlog
mentioned in issue #208670 (closed)
added 726 commits
-
2c71c315...fb46b18b - 725 commits from branch
master
- 870424b6 - Only make Review App deployment automatic for frontend changes
-
2c71c315...fb46b18b - 725 commits from branch
removed workflowblocked label
changed milestone to %12.10
- Resolved by Rémy Coutable
@caalberts @godfat Could you please review? Thanks!
assigned to @caalberts and @godfat and unassigned @rymai
- Resolved by Rémy Coutable
- Resolved by Rémy Coutable
@rymai Thanks, I have comments on the frontend-patterns.
Also, I'm assuming with this change we would have less MRs going through
review-qa-smoke
,review-performance
anddast
. I didn't see this being acknowledged anywhere in the issue, so just wanted to clarify that we're ok with it.In any case, they have been running with
allow_failure: true
and dependent on successful review deploy, so they're not a blocking gate.Edited by Albert Salim
assigned to @rymai and unassigned @caalberts
- Resolved by Rémy Coutable
added 449 commits
-
870424b6...2248a378 - 448 commits from branch
master
- f6cacff6 - Only make Review App deployment automatic for frontend changes
-
870424b6...2248a378 - 448 commits from branch
added 1 commit
- 347cdade - Only make Review App deployment automatic for frontend changes
- Resolved by Rémy Coutable
- Resolved by Rémy Coutable
- Resolved by Rémy Coutable
@rymai Thanks! I agree with the existing comments and just left two more minor thoughts about the names of rules.
unassigned @godfat
- Resolved by Rémy Coutable
- Resolved by Rémy Coutable
added 91 commits
-
347cdade...99b76451 - 90 commits from branch
master
- 1b037a29 - Only make Review App deployment automatic for frontend changes
-
347cdade...99b76451 - 90 commits from branch
added backstage [DEPRECATED] label
- Resolved by Lin Jen-Shin
@godfat Thanks for the review! Please have another look!
I plan to document this as a follow-up.
- Resolved by Lin Jen-Shin
- Resolved by Lin Jen-Shin
added 1 commit
- 44fa3382 - Only make Review App deployment automatic for frontend changes
- Resolved by Lin Jen-Shin
enabled an automatic merge when the pipeline for 287976d5 succeeds
mentioned in commit 80710af9
added workflowstaging label
mentioned in issue gitlab-com/www-gitlab-com#6326 (closed)
added workflowcanary label and removed workflowstaging label
mentioned in issue #211579 (closed)
added workflowproduction label and removed workflowcanary label
mentioned in issue #205307 (closed)
mentioned in merge request !27926 (closed)
mentioned in issue #30140 (closed)