Gerrit/Privilege policy

This is a draft of a new Gerrit privilege policy, which merges Gerrit/+2 and Gerrit/Project_ownership with some changes.

Things to change, based on TechCom feedback:


 * Clarify deployment branch section. Recommend +1 be sought in puppet etc., instead of merging changes without review. Clarify/add backport policy.
 * Add revocation for behaviour reasons.
 * Explain how a revocation request for behaviour or competency reasons should be approached, when such issues are noticed by a developer.
 * Clarify what is meant by a WMF extension.
 * Recommend that maintainers of non-WMF extensions should no longer have the ability to add group members without review. Instead maintainers nominate new developers via the normal process. In order to avoid a dominictarr type scenario.
 * Ensure that if a non-WMF extension is deployed by WMF, Gerrit ownership is properly secured.

Rights and responsibilities
Most Wikimedia Gerrit repositories are configured such that when a user gives a "+2" code review, Jenkins will automatically merge the change after running tests. So the right to give a +2 code review is restricted to people in particular Gerrit groups. Typically, there is a Gerrit group named after each repository which grants privileges on that repository. Also, some Gerrit groups grant privileges on multiple repositories. Most notably, the mediawiki group grants privileges on the MediaWiki core and all extensions and skins, as well as some other MediaWiki-related repositories.

Gerrit groups can derive their members from LDAP groups. Most WMF employees are in the wmf group in LDAP, which automatically grants them membership of the mediawiki group.

Gerrit administrators are able to add and remove members from Gerrit groups.

Merging a change to the MediaWiki core or an extension deployed by Wikimedia is a big deal. The change will be automatically deployed to the beta cluster, a virtualized staging environment, as soon as it's been merged in Gerrit. It will also be automatically picked up in the next MediaWiki core deployment window (see Deployments) unless it is reverted before the release branch is cut.

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 using +2.

+2 is 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 +2 power have a special responsibility to see from others' points of view.

Self-review
Self-review is bad for code quality and bad for morale. The point of +2 rights is to separate development and code review. The purpose of merging a change in Gerrit is to tell the world that "Yes, I've ensured that this change follows MediaWiki conventions, good engineering practices and is sane." (Cf. "Code Reviews: Just Do It" by Jeff Atwood.) Inline comments can be used to indicate issues with the code that should be addressed before it is merged. Exercising -1 or -2 is just as important as +2.


 * Trivial changes : Very few changes are trivial enough to self-merge. Self-merging is tolerated in some cases like trivial documentation changes or projects with only one maintainer. In projects deployed on the Wikimedia cluster, +2-ing your own code is unacceptable and can be grounds for revocation.
 * Reverts : Reverts can generally be self-merged as long as the commit it is reverting was recent.
 * Deployment branches : The puppet and mediawiki-config repositories are typically deployed directly from master (called production in puppet) by the author of the change. It's a nuisance to discover multiple pending changes when deploying from these repositories, so deployment is usually done immediately after merge. Self-merge is allowed in these repositories. Code review is usually indicated with "+1".
 * Non-Wikimedia extensions : For extensions (and other projects) not used by Wikimedia, the code review policy is up to the maintainer or author of the extension. Some non-Wikimedia extensions follow Wikimedia's policy of prohibiting self-merges, but there is no requirement of that. If you are the only person writing the extension and have nobody to review your change, or if the extension is abandoned, it is acceptable to self-merge your changes.

Intra-team review and shared ownership
If you're working as part of a team, review by members of your team are not only permitted, but strongly encouraged. Having peers review your code on an ongoing basis is a great way to keep momentum of development going, and ensure that your reviewers are familiar with what you're doing.

When you're doing intra-team review, be especially sensitive about blind spots, cognitive biases, and the need to get buy-in for large changes outside the group of people you're working in. Most open source projects, including MediaWiki, are full of abandoned efforts to create fancy new abstraction layers, skinning systems, testing frameworks, etc. Consider the impact of your changes on the ecosystem as a whole, and engage in conversations through requests for comment, wikitech-l, IRC and other venues. Shared ownership of code (to a greater or lesser degree) helps to ensure that what you're doing has lasting long term value.

Must read for code reviewers

 * Code review guide
 * Security for developers
 * Manual:Coding conventions (and subpages)
 * Manual:Unit testing (and related pages)
 * Localisation
 * Database optimization - keep this in mind when making schema changes (which should be implemented following this process)
 * Manual:How to debug

Requesting Gerrit privileges
To request membership in the mediawiki group, create a new task under the MediaWiki-Group-Requests project in Phabricator. Then send an email to wikitech-l.

To request membership in another group, create a new task under the Gerrit-Privilege-Requests project in Phabricator.

In either case, the Phabricator task should specify:


 * Gerrit username
 * repository or repositories to which access is required
 * reasoning, including links to patches written and reviewed

Developers commenting on a privilege request should consider whether the applicant has contributed high quality patches, has exercised +1 rights well, and has demonstrated competence. Negative comments should be written with tact, they should not be overly strident.

If there is a consensus of trusted developers on the Phabricator task, any of the Gerrit administrators can resolve the request. The task must remain open for at least a week, to allow interested developers to comment. Additional time should be allowed if the request is open during travel or holiday periods.

If there is no consensus on a request in Phabricator, it may be referred to TechCom for adjudication.

Expedited process for trusted organisations
Gerrit administrators may immediately act on requests from the following trusted organisations for the granting and revocation of related groups, specifically:

It is not necessary to file a Phabricator task or demonstrate consensus.

This facility is intended to allow these organisations to rapidly on-board staff members, who are assumed to be trusted by virtue of the hiring process. It also allows trusted organisations to grant access to volunteers who are well known and trusted by those organisations.

WMF employees may be added to the wmf group in LDAP when they are hired. Wikimedia Deutschland employees may be added to the wmde group in LDAP when they are hired.

TechCom, or the CTO in consultation with TechCom, may direct a Gerrit administrator to add any person to a Gerrit group.

Revocation
Revocation of Gerrit rights is permitted in the following circumstances:


 * In an emergency, such as a compromised account, Gerrit administrators may revoke access immediately, at their discretion. Reversal of emergency revocation may be done at the administrator's discretion if the emergency is judged to have passed. TechCom, or the CTO in consultation with TechCom, may review an emergency revocation and direct its reversal.
 * Revocation of privileges of a WMF employee may be directed by that employee's manager, in consultation with WMF Talent and Culture, as discussed in the Staff Handbook.
 * Revocation of privileges from any person may be directed by TechCom, or by the CTO in consultation with TechCom.

The reasons for revocation may include:


 * Repeatedly merging bad code
 * Repeatedly merging your own code
 * Failing to socialize high impact changes within the development community
 * Not following the guidelines above
 * Termination of employment or contract

It is WMF policy to revoke all privileges when staff members depart, even if those privileges were granted prior to the beginning of employment by virtue of volunteer work. Consistent application of this policy helps to protect the privacy of departing staff members: no fault is implied. If departed staff members wish to continue to contribute in a volunteer capacity, they may reapply for access by the usual process.

Amendments to this policy
Amendments to this policy must be approved by the CTO, in consultation with TechCom.

Definitions

 * CTO : The Chief Technology Officer of the Wikimedia Foundation.
 * TechCom: The Wikimedia Technical Committee. For TechCom to decide or direct something means that a meeting of the committee, by remote audio/video conference or in person, with the attendance of a quorum of at least half of TechCom's members, passes a resolution by simple majority or unanimous consent.