Merge branch 'docs-merge-requests' into 'master'

Docs: Cleaning up the merge request workflow

Closes #51355

See merge request gitlab-org/gitlab-ce!24559

doc/development/contributing/index.md

@@ -119,6 +119,7 @@ This [documentation](merge_request_workflow.md) outlines the current merge reque
 - [Merge request guidelines](merge_request_workflow.md#merge-request-guidelines)
 - [Contribution acceptance criteria](merge_request_workflow.md#contribution-acceptance-criteria)
 - [Definition of done](merge_request_workflow.md#definition-of-done)
+- [Dependencies](merge_request_workflow.md#dependencies)
 
 ## Style guides
 

doc/development/contributing/merge_request_workflow.md

 # Merge requests
 
-We welcome merge requests with fixes and improvements to GitLab code, tests,
-and/or documentation. The issues that are specifically suitable for
-community contributions are listed with
-[the `Accepting merge requests` label](issue_workflow.md#label-for-community-contributors),
-but you are free to contribute to any other issue you want.
-
-Please note that if an issue is marked for the current milestone either before
-or while you are working on it, a team member may take over the merge request
+We welcome merge requests from everyone, with fixes and improvements
+to GitLab code, tests, and documentation. The issues that are specifically suitable
+for community contributions are listed with the [`Accepting merge requests`](issue_workflow.md#label-for-community-contributors)
+label, but you are free to contribute to any issue you want.
+
+Please note that if an issue is marked for the current milestone at any time, even
+when you are working on it, a GitLab Inc. team member may take over the merge request
 in order to ensure the work is finished before the release date.
 
-If you want to add a new feature that is not labeled it is best to first create
-a feedback issue (if there isn't one already) and leave a comment asking for it
+If you want to add a new feature that is not labeled, it is best to first create
+an issue (if there isn't one already) and leave a comment asking for it
 to be marked as `Accepting Merge Requests`. Please include screenshots or
-wireframes if the feature will also change the UI.
+wireframes of the proposed feature if it will also change the UI.
 
-Merge requests should be opened at [GitLab.com][gitlab-mr-tracker].
+Merge requests should be submitted to the appropriate project at GitLab.com, for example
+[GitLab CE](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests),
+[GitLab EE](https://gitlab.com/gitlab-org/gitlab-ee/merge_requests),
+[GitLab Runner](https://gitlab.com/gitlab-org/gitlab-runner/merge_requests),
+[GitLab Omnibus](https://gitlab.com/gitlab-org/omnibus-gitlab/merge_requests), etc.
 
 If you are new to GitLab development (or web development in general), see the
-[I want to contribute!](index.md#i-want-to-contribute) section to get you started with
+[I want to contribute!](index.md#i-want-to-contribute) section to get started with
 some potentially easy issues.
 
-To start with GitLab development download the [GitLab Development Kit][gdk] and
-see the [Development section](../../README.md) for some guidelines.
-
-[gitlab-mr-tracker]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests
-[gdk]: https://gitlab.com/gitlab-org/gitlab-development-kit
+To start developing GitLab, download the [GitLab Development Kit](https://gitlab.com/gitlab-org/gitlab-development-kit)
+and see the [Development section](../../README.md) for the required guidelines.
 
 ## Merge request guidelines
 
-If you can, please submit a merge request with the fix or improvements
-including tests. If you don't know how to fix the issue but can write a test
-that exposes the issue we will accept that as well. In general bug fixes that
-include a regression test are merged quickly while new features without proper
-tests are least likely to receive timely feedback. The workflow to make a merge
+If you find an issue, please submit a merge request with a fix or improvement, if
+you can, and include tests. If you don't know how to fix the issue but can write a test
+that exposes the issue, we will accept that as well. In general, bug fixes that
+include a regression test are merged quickly, while new features without proper
+tests might be slower to receive feedback. The workflow to make a merge
 request is as follows:
 
-1. Fork the project into your personal space on GitLab.com
-1. Create a feature branch, branch away from `master`
-1. Write [tests](https://docs.gitlab.com/ee/development/rake_tasks.html#run-tests) and code
-1. [Generate a changelog entry with `bin/changelog`][changelog]
+1. [Fork](../../workflow/forking_workflow.md#creating-a-fork) the project into
+   your personal namespace (or group) on GitLab.com.
+1. Create a feature branch in your fork (don't work off `master`).
+1. Write [tests](../rake_tasks.md#run-tests) and code.
+1. [Generate a changelog entry with `bin/changelog`](../changelog.md)
 1. If you are writing documentation, make sure to follow the
-   [documentation guidelines][doc-guidelines]
-1. If you have multiple commits please combine them into a few logically
-  organized commits by [squashing them][git-squash]
-1. Push the commit(s) to your fork
-1. Submit a merge request (MR) to the `master` branch
-   1. Your merge request needs at least 1 approval but feel free to require more.
-      For instance if you're touching backend and frontend code, it's a good idea
+   [documentation guidelines](../documentation/index.md).
+1. Follow the [commit messages guidelines](#commit-messages-guidelines).
+1. If you have multiple commits, combine them into a few logically organized
+   commits by [squashing them](https://git-scm.com/book/en/v2/Git-Tools-Rewriting-History#_squashing),
+   but do not change the commit history if you're working on shared branches though.
+1. Push the commit(s) to your working branch in your fork.
+1. Submit a merge request (MR) to the `master` branch in the main GitLab project.
+   1. Your merge request needs at least 1 approval, but feel free to require more.
+      For instance if you're touching both backend and frontend code, it's a good idea
       to require 2 approvals: 1 from a backend maintainer and 1 from a frontend
-      maintainer
-   1. You don't have to select any approvers, but you can if you really want
-      specific people to approve your merge request
-1. The MR title should describe the change you want to make
-1. The MR description should give a motive for your change and the method you
-   used to achieve it.
-   1. If you are contributing code, fill in the template already provided in the
-      "Description" field.
+      maintainer.
+   1. If you're submitting changes to documentation, you'll need approval from a technical
+      writer, based on the appropriate [product category](https://about.gitlab.com/handbook/product/categories/).
+      Only assign the MR to them when it's ready for docs review.
+   1. You don't have to select any specific approvers, but you can if you really want
+      specific people to approve your merge request.
+1. The MR title should describe the change you want to make.
+1. The MR description should give a reason for your change.
+   1. If you are contributing code, fill in the description according to the default
+      template already provided in the "Description" field.
    1. If you are contributing documentation, choose `Documentation` from the
-      "Choose a template" menu and fill in the template.
+      "Choose a template" menu and fill in the description according to the template.
    1. Mention the issue(s) your merge request solves, using the `Solves #XXX` or
-      `Closes #XXX` syntax to auto-close the issue(s) once the merge request will
-      be merged.
-1. If you're allowed to, set a relevant milestone and labels
-1. If the MR changes the UI it should include *Before* and *After* screenshots
-1. If the MR changes CSS classes please include the list of affected pages,
-   `grep css-class ./app -R`
-1. Be prepared to answer questions and incorporate feedback even if requests
-   for this arrive weeks or months after your MR submission
-   1. If a discussion has been addressed, select the "Resolve discussion" button
-      beneath it to mark it resolved.
-1. If your MR touches code that executes shell commands, reads or opens files or
+      `Closes #XXX` syntax to [auto-close](../../user/project/issues/automatic_issue_closing.md)
+      the issue(s) once the merge request is merged.
+1. If you're allowed to (Core team members, for example), set a relevant milestone
+   and [labels](issue_workflow.md).
+1. If the MR changes the UI, it should include *Before* and *After* screenshots.
+1. If the MR changes CSS classes, please include the list of affected pages, which
+   can be found by running `grep css-class ./app -R`.
+1. Be prepared to answer questions and incorporate feedback into your MR with new
+   commits. Once you have fully addressed a suggestion from a reviewer, click the
+   "Resolve discussion" button beneath it to mark it resolved.
+   1. The merge request author resolves only the discussions they have fully addressed.
+      If there's an open reply or discussion, a suggestion, a question, or anything else,
+      the discussion should be left to be resolved by the reviewer.
+1. If your MR touches code that executes shell commands, reads or opens files, or
    handles paths to files on disk, make sure it adheres to the
    [shell command guidelines](../shell_commands.md)
 1. If your code creates new files on disk please read the
    [shared files guidelines](../shared_files.md).
-1. When writing commit messages please follow
-   [these](http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html)
-   [guidelines](http://chris.beams.io/posts/git-commit/).
 1. If your merge request adds one or more migrations, make sure to execute all
    migrations on a fresh database before the MR is reviewed. If the review leads
-   to large changes in the MR, do this again once the review is complete.
-1. For more complex migrations, write tests.
-1. Merge requests **must** adhere to the [merge request performance
-   guidelines](../merge_request_performance_guidelines.md).
-1. For tests that use Capybara or PhantomJS, see this [article on how
-   to write reliable asynchronous tests](https://robots.thoughtbot.com/write-reliable-asynchronous-integration-tests-with-capybara).
+   to large changes in the MR, execute the migrations again once the review is complete.
+1. Write tests for more complex migrations.
+1. Merge requests **must** adhere to the [merge request performance guidelines](../merge_request_performance_guidelines.md).
+1. For tests that use Capybara, read
+   [how to write reliable, asynchronous integration tests](https://robots.thoughtbot.com/write-reliable-asynchronous-integration-tests-with-capybara).
 1. If your merge request introduces changes that require additional steps when
    installing GitLab from source, add them to `doc/install/installation.md` in
    the same merge request.
@@ -95,109 +99,117 @@ request is as follows:
    instructions are specific to a version, add them to the "Version specific
    upgrading instructions" section.
 
-Please keep the change in a single MR **as small as possible**. If you want to
-contribute a large feature think very hard what the minimum viable change is.
-Can you split the functionality? Can you only submit the backend/API code? Can
-you start with a very simple UI? Can you do part of the refactor? The increased
-reviewability of small MRs that leads to higher code quality is more important
-to us than having a minimal commit log. The smaller an MR is the more likely it
-is it will be merged (quickly). After that you can send more MRs to enhance it.
-The ['How to get faster PR reviews' document of Kubernetes](https://github.com/kubernetes/community/blob/master/contributors/devel/faster_reviews.md) also has some great points regarding this.
-
-For examples of feedback on merge requests please look at already
-[closed merge requests][closed-merge-requests]. If you would like quick feedback
-on your merge request feel free to mention someone from the [core team] or one
-of the [Merge request coaches][team].
-Please ensure that your merge request meets the contribution acceptance criteria.
-
-When having your code reviewed and when reviewing merge requests please take the
-[code review guidelines](../code_review.md) into account.
-
-[git-squash]: https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits
-[closed-merge-requests]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests?assignee_id=&label_name=&milestone_id=&scope=&sort=&state=closed
-[team]: https://about.gitlab.com/team/
+If you would like quick feedback on your merge request feel free to mention someone
+from the [core team](https://about.gitlab.com/community/core-team/) or one of the
+[merge request coaches](https://about.gitlab.com/team/). When having your code reviewed
+and when reviewing merge requests, please keep the [code review guidelines](../code_review.md)
+in mind.
+
+### Keep it simple
+
+*Live by smaller iterations.* Please keep the amount of changes in a single MR **as small as possible**.
+If you want to contribute a large feature, think very carefully about what the
+[minimum viable change](https://about.gitlab.com/handbook/product/#the-minimally-viable-change)
+is. Can you split the functionality into two smaller MRs? Can you submit only the
+backend/API code? Can you start with a very simple UI? Can you do just a part of the
+refactor?
+
+Small MRs which are more easily reviewed, lead to higher code quality which is
+more important to GitLab than having a minimal commit log. The smaller an MR is,
+the more likely it will be merged quickly. After that you can send more MRs to
+enhance and expand the feature. The [How to get faster PR reviews](https://github.com/kubernetes/kubernetes/blob/release-1.5/docs/devel/faster_reviews.md)
+document from the Kubernetes team also has some great points regarding this.
+
+### Commit messages guidelines
+
+When writing commit messages, please follow the guidelines below:
+
+- The commit subject must contain at least 3 words.
+- The commit subject should ideally contain up to 50 characters,
+and must not be longer than 72 characters.
+- The commit subject must start with a capital letter.
+- The commit subject must not end with a period.
+- The commit subject and body must be separated by a blank line.
+- The commit body must not contain more than 72 characters per line.
+- Commits that change 30 or more lines across at least 3 files must
+describe these changes in the commit body.
+- The commit subject or body must not contain Emojis.
+- Use issues and merge requests' full URLs instead of short references,
+as they are displayed as plain text outside of GitLab.
+- The merge request must not contain more than 10 commit messages.
+
+If the guidelines are not met, the MR will not pass the
+[Danger checks](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/danger/commit_messages/Dangerfile).
+For more information see [How to Write a Git Commit Message](https://chris.beams.io/posts/git-commit/).
 
 ## Contribution acceptance criteria
 
-1. The change is as small as possible
+To make sure that your merge request can be approved, please ensure that it meets
+the contribution acceptance criteria below:
+
+1. The change is as small as possible.
 1. Include proper tests and make all tests pass (unless it contains a test
    exposing a bug in existing code). Every new class should have corresponding
    unit tests, even if the class is exercised at a higher level, such as a feature test.
-1. If you suspect a failing CI build is unrelated to your contribution, you may
-   try and restart the failing CI job or ask a developer to fix the
-   aforementioned failing test
-1. Your MR initially contains a single commit (please use `git rebase -i` to
-   squash commits)
-1. Your changes can merge without problems (if not please rebase if you're the
-   only one working on your feature branch, otherwise, merge `master`)
-1. Does not break any existing functionality
-1. Fixes one specific issue or implements one specific feature (do not combine
-   things, send separate merge requests if needed)
-1. Migrations should do only one thing (e.g., either create a table, move data
-   to a new table or remove an old table) to aid retrying on failure
-1. Keeps the GitLab code base clean and well structured
-1. Contains functionality we think other users will benefit from too
-1. Doesn't add configuration options or settings options since they complicate
-   making and testing future changes
-1. Changes do not adversely degrade performance.
-   - Avoid repeated polling of endpoints that require a significant amount of overhead
-   - Check for N+1 queries via the SQL log or [`QueryRecorder`](https://docs.gitlab.com/ce/development/merge_request_performance_guidelines.html)
-   - Avoid repeated access of filesystem
-1. If you need polling to support real-time features, please use
-   [polling with ETag caching][polling-etag].
-1. Changes after submitting the merge request should be in separate commits
-   (no squashing).
-1. It conforms to the [style guides](style_guides.md) and the following:
-    - If your change touches a line that does not follow the style, modify the
-      entire line to follow it. This prevents linting tools from generating warnings.
-    - Don't touch neighbouring lines. As an exception, automatic mass
-      refactoring modifications may leave style non-compliant.
-1. If the merge request adds any new libraries (gems, JavaScript libraries,
-   etc.), they should conform to our [Licensing guidelines][license-finder-doc].
-   See the instructions in that document for help if your MR fails the
-   "license-finder" test with a "Dependencies that need approval" error.
-1. The merge request meets the [definition of done](#definition-of-done).
-
-[license-finder-doc]: ../licensing.md
-[polling-etag]: ../polling.md
+   - If a failing CI build seems to be unrelated to your contribution, you can try
+     restarting the failing CI job, rebasing from master to bring in updates that
+     may resolve the failure, or if it has not been fixed yet, ask a developer to
+     help you fix the test.
+1. The MR initially contains a a few logically organized commits.
+1. The changes can merge without problems. If not, you should rebase if you're the
+   only one working on your feature branch, otherwise merge `master`.
+1. Only one specific issue is fixed or one specific feature is implemented. Do not
+   combine things; send separate merge requests for each issue or feature.
+1. Migrations should do only one thing (e.g., create a table, move data to a new
+   table, or remove an old table) to aid retrying on failure.
+1. Contains functionality that other users will benefit from.
+1. Doesn't add configuration options or settings options since they complicate making
+   and testing future changes.
+1. Changes do not degrade performance:
+   - Avoid repeated polling of endpoints that require a significant amount of overhead.
+   - Check for N+1 queries via the SQL log or [`QueryRecorder`](../merge_request_performance_guidelines.md).
+   - Avoid repeated access of the filesystem.
+   - Use [polling with ETag caching](../polling.md) if needed to support real-time features.
+1. If the merge request adds any new libraries (gems, JavaScript libraries, etc.),
+   they should conform to our [Licensing guidelines](../licensing.md). See those
+   instructions for help if the "license-finder" test fails with a
+   `Dependencies that need approval` error. Also, make the reviewer aware of the new
+   library and explain why you need it.
+1. The merge request meets GitLab's [definition of done](#definition-of-done), below.
 
 ## Definition of done
 
 If you contribute to GitLab please know that changes involve more than just
-code. We have the following [definition of done][definition-of-done]. Please ensure you support
-the feature you contribute through all of these steps.
-
-1. Description explaining the relevancy (see following item)
-1. Working and clean code that is commented where needed
-1. [Unit, integration, and system tests][testing] that pass on the CI server
-1. Performance/scalability implications have been considered, addressed, and tested
-1. [Documented][doc-guidelines] in the `/doc` directory
-1. [Changelog entry added][changelog], if necessary
-1. Reviewed by UX/FE/BE and any concerns are addressed
-1. Merged by a project maintainer
-1. Added to the release blog article, if relevant
-1. Added to [the website](https://gitlab.com/gitlab-com/www-gitlab-com/), if relevant
-1. Community questions answered
-1. Answers to questions radiated (in docs/wiki/support etc.)
-1. [Black-box tests/end-to-end tests](../testing_guide/testing_levels.md#black-box-tests-at-the-system-level-aka-end-to-end-tests) added if required. Please contact [the quality team](https://about.gitlab.com/handbook/engineering/quality/#teams) with any questions
+code. We use the following [definition of done](https://www.agilealliance.org/glossary/definition-of-done).
+Your contribution is not *done* until you have made sure it meets all of these
+requirements.
+
+1. Clear description explaining the relevancy of the contribution.
+1. Working and clean code that is commented where needed.
+1. [Unit, integration, and system tests](../testing_guide/index.md) that all pass
+   on the CI server.
+1. Performance/scalability implications have been considered, addressed, and tested.
+1. [Documented](../documentation/index.md) in the `/doc` directory.
+1. [Changelog entry added](../changelog.md), if necessary.
+1. Reviewed by relevant (UX/FE/BE/tech writing) reviewers and all concerns are addressed.
+1. Merged by a project maintainer.
+1. Added to the [release post](https://about.gitlab.com/handbook/marketing/blog/release-posts/),
+   if relevant.
+1. Added to [the website](https://gitlab.com/gitlab-com/www-gitlab-com/blob/master/data/features.yml), if relevant.
+1. [Black-box tests/end-to-end tests](../testing_guide/testing_levels.md#black-box-tests-at-the-system-level-aka-end-to-end-tests)
+   added if required. Please contact [the quality team](https://about.gitlab.com/handbook/engineering/quality/#teams)
+   with any questions.
+
+## Dependencies
 
 If you add a dependency in GitLab (such as an operating system package) please
-consider updating the following and note the applicability of each in your
-merge request:
-
-1. Note the addition in the release blog post (create one if it doesn't exist yet) <https://gitlab.com/gitlab-com/www-gitlab-com/merge_requests/>
-1. Upgrade guide, for example <https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/update/7.5-to-7.6.md>
-1. Installation guide <https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/install/installation.md#1-packages-dependencies>
-1. GitLab Development Kit <https://gitlab.com/gitlab-org/gitlab-development-kit>
-1. Test suite <https://gitlab.com/gitlab-org/gitlab-ce/blob/master/scripts/prepare_build.sh>
-1. Omnibus package creator <https://gitlab.com/gitlab-org/omnibus-gitlab>
-
-[definition-of-done]: http://guide.agilealliance.org/guide/definition-of-done.html
-[testing]: ../testing_guide/index.md
-
----
-
-[Return to Contributing documentation](index.md)
-
-[changelog]: ../changelog.md "Generate a changelog entry"
-[doc-guidelines]: ../documentation/index.md "Documentation guidelines"
+consider updating the following, and note the applicability of each in your merge
+request:
+
+1. Note the addition in the [release blog post](https://about.gitlab.com/handbook/marketing/blog/release-posts/)
+   (create one if it doesn't exist yet).
+1. [The upgrade guide](../../update/upgrading_from_source.md).
+1. The [GitLab Installation Guide](../../install/installation.md#1-packages-and-dependencies).
+1. The [GitLab Development Kit](https://gitlab.com/gitlab-org/gitlab-development-kit).
+1. The [CI environment preparation](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/scripts/prepare_build.sh).
+1. The [Omnibus package creator](https://gitlab.com/gitlab-org/omnibus-gitlab).