Consider moving the `danger-review` job before the `prepare` stage
Benefits
- This saves a lot of money by not running expensive jobs when we can quickly know that the pipeline won't pass. This is very important and in line with gitlab-com/www-gitlab-com#6610 (comment 302762602) / &1853.
- As can be seen in https://app.periscopedata.com/app/gitlab/496118/Engineering-Productivity-Sandbox?widget=8100542&udv=785399 there are between 50 and 100 failed
danger-review
jobs per day, and agitlab
pipeline costs us 4$ (https://gitlab.com/gitlab-org/gitlab/-/issues/210601). Running 8 jobs instead of 203 in these cases can save a lot of money (e.g. even if the 8 jobs would cost 1$, we'd save between50*(4-1)*20(working days) = 3000$
and100*(4-1)*20(working days) = 6000$
). - Of course, with this change, some of these pipelines would be fixed and a new pipeline would be started because authors need other jobs to run (e.g. tests or Review Apps), but in the cases where people don't really look at pipelines output or when a MR is ready to be merged, that would avoid unnecessarily running many jobs.
- As can be seen in https://app.periscopedata.com/app/gitlab/496118/Engineering-Productivity-Sandbox?widget=8100542&udv=785399 there are between 50 and 100 failed
- This gives a quicker feedback (approximately 25 minutes since
gitlab:assets:compile pull-cache
starts immediately) to engineers when their MR fails any Danger checks.
Drawbacks
- Some people would argue that when their MR isn't "ready to be merged", they'd prefer to have the pipeline run so that they can gather all the failures in one go.
Ultimately, if I am actively working on something, I don't care about anything Danger complains about. I care only about working code (which MR labels and commit messages are not a part of). I view Dangerbot as a "fix before assigning for review", as they are essential runs for reviewers, maintainers, and metrics.
Danger fails on translations, for instance. But I generate those last because the text might change as we work through a feature and I want to just run it once. Having to always have new translations generated in order to be sure the tests are passing as you add new code can lead to more cycles. Likewise, consider the interaction with the 10-commit rule, review suggestions, and people who want to rebase once at the end.
Edited by Rémy Coutable