Skip to content

Draft: Remove danger customization already in template

João Alexandre Cunha requested to merge jcunha-fix-roulette into master

What does this MR do?

In !3319 (merged), when attempting to introduce the roulette for the 2nd time, the first commit in the MR did not include the custom danger-review job with the new script tag. So, the script tag got inherited from the danger-review template. Consequently, the Danger job worked and printed the comment in the MR with the roulette. All good up to that point. In the following commit, we've added the job to introduce the no-op script tag, so that we could fix the dev environment. This indeed fixed the dev environment. But it silently "failed" the Danger job for .com, because the job passed on .com, and we had a Danger comment on the MR, which was the first initial comment from the previous commit, giving a false impression that everything was correct in .com.

We believed that by adding the job definition with the script: "no-op" before including the template would work, as our CI overrides jobs in favour of ones that are defined last. But what happened is that the template, even being defined afterwards, was parsed before the job definition. So it looks like we execute template includes even before we define jobs. 🤷

But the truth is, the only reason we were defining a job was to add the gitlab-dangerfiles gem to this job. Another solution to not have to define the job at all, is to include this gem directly on the Gemfile and allow the imported template to install the gem from the Gemfile.

Related issues

Related: gitlab-org/distribution/team-tasks#1304 (closed)

Checklist

See Definition of done.

For anything in this list which will not be completed, please provide a reason in the MR discussion.

Required

  • Merge Request Title and Description are up to date, accurate, and descriptive
  • MR targeting the appropriate branch
  • MR has a green pipeline on GitLab.com
  • When ready for review, follow the instructions in the "Reviewer Roulette" section of the Danger Bot MR comment, as per the Distribution experimental MR workflow

Expected (please provide an explanation if not completing)

  • Test plan indicating conditions for success has been posted and passes
  • Documentation created/updated
  • Tests added
  • Integration tests added to GitLab QA
  • Equivalent MR/issue for omnibus-gitlab opened
  • Validate potential values for new configuration settings. Formats such as integer 10, duration 10s, URI scheme://user:passwd@host:port may require quotation or other special handling when rendered in a template and written to a configuration file.
Edited by João Alexandre Cunha

Merge request reports