GitLab/Workflows/Code review

= Reviewing code on GitLab =

This document is a work in progress.

Review before merge
It's important to us to have a review-before-merge workflow for MediaWiki core and also for any extension we deploy.

Who can review and merge code in GitLab
Any user should be able to comment on a merge request, however only users with Developer- or Maintainer-level permissions for a given repository can merge code.

Self-merge is discouraged by policy. Merging your own code without approval from a reviewer may be grounds for revocation of privileges.

Merging a change to the MediaWiki core or an extension deployed by Wikimedia is a big deal. Your merge could cause Wikipedia or other sites to fail. It could create a security vulnerability that allows attackers to delete or corrupt data, or to gain access to private information. And in the more common case, it could cause technical debt to increase if the code doesn't have tests, is poorly implemented or poorly socialized. You should carefully read this document and all relevant policies before merging a merge request.

Maintainer permissions are a strong expression of trust, and trust is maintained through good judgement and careful action.

In code review, design discussions, and bug comments, those with merge abilities have a special responsibility to see from others' points of view.

Comments and threads
Gitlab, like Gerrit, supports threaded discussion. Replies can be linked with a comment, and a whole thread can be marked as resolved. On Gerrit, resolving/unresolving a thread is only possible when commenting on it, and so it is conventional to resolve a thread with a comment “Done” if there is nothing else to say. On GitLab, on the other hand, this is not necessary: A thread can be marked as resolved or unresolved without requiring a new comment.

Inline suggestions
GitLab comments support inline suggestions for simple changes. This is a powerful feature, particularly for small, uncontroversial changes such as typo-fixes or style tweaks.

As a reviewer, you can add a suggestion using the following syntax (also available by clicking the "Insert suggestion" button on the comment formatting toolbar):

```suggestion:-0+0 replacement text here ```

Best practices:


 * Use inline suggestions for minor changes (typos, style issues, clarifying comments).
 * Multiple suggestions should generally be applied as a batch.
 * A merge request with accepted suggestions should typically be squashed on merge in order to avoid noisy commits.