Skip to content

Simplify our MR template

João Alexandre Cunha requested to merge docs-Alexand-master-patch-64177 into master

What does this MR do?

Clean up our MR template

It's clear when reviewing our MRs that much of our checklist is being neglected by authors, specially the "Expected" section.

I believe that the current list is quite long, so it's been demotivating for people to go through all of it. My impression is that people only follow the "Required" section. This commit proposes the following changes:

  1. Remove the Expected sections.
  2. Move the ask to justify incomplete tasks to the Required section.
  3. Change 'MR has a green pipeline on GitLab.com' to 'MR has a green pipeline.', because forked MR authors can't test the full pipeline on GitLab.com.
  4. Move documentation guidance to the required section.
  5. Move Tests added/updated to the required section.
  6. Merge Tests added/updated with the test plan guidance.
  7. Remove the suggestion to add GitLab QA tests. I don't know of anyone who's ever did this, and it's a big task.
  8. Remove the equivalent Operator MR. It's hard for most authors to identify this. The most realistic solution will be to trigger a downstream pipeline to test it, which we want to plan for the next release.
  9. Testing the operator is quite cumbersome, specially for community contributors, so asking this from authors is a bit pushy. So, we're creating a "Reviewers" section to consider Operator changes.
  10. The new section for "Reviewers" will also the reviewer to validate the pipeline is green, not in GitLab.com, but in https://gitlab.com/gitlab-org/charts/gitlab, which is more specific.
  11. Instead of linking the users for just the Definition of Done, we point them to the whole CONTRIBUTING.md.
  12. Remove the Validate potential values for new configuration settings... This is a development guideline. I think could actually point the users to many other development best practices. I'm not sure that specifying one single best practice makes sense? If anything, we can link them to an entrypoint on best practices. Which should be probably part of the CONTRIBUTE.md?
  13. We're removing the community fork paragraph in favor of implementing bot automation to talk about, as done already for gitlab-org/gitlab, ex:

gitlab-org/gitlab!154517 (comment 1926427908)

Related issues

Author 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

For merge requests from forks, consider the following options for Danger to work properly:

Expected (please provide an explanation if not completing)

  • Test plan indicating conditions for success has been posted and passes
  • Documentation created/updated
  • Tests added/updated
  • Integration tests added to GitLab QA
  • Equivalent MR/issue for omnibus-gitlab opened
  • Equivalent MR/issue for Gitlab Operator project opened (see Operator documentation on impact of Charts changes)
  • 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