|This page documents a MediaWiki development policy, crafted over time by developer consensus (or sometimes by proclamation from a lead developer).|
For the context this page refers to a permission in Wikimedia Foundation code review system (Gerrit). Any change submitted is awarded a code review score between -2 (do not submit) and +2. This guideline is about the latter.
So, you got +2 rights to MediaWiki core or a WMF-deployed extension.
That means you can merge changes to the software that's running on Wikimedia Foundation sites. Your 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 wikitech:Deployments) unless it is reverted before the release branch is cut.
This 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're therefore required to read this entire document and carefully review all the relevant links in it before using +2.
+2 is a strong expression of trust, and trust is maintained through good judgment and careful action.
- 1 Self-merge
- 2 +2 is for things you're qualified to review
- 3 Social conduct
- 4 Must read for code reviewers
- 5 Pair programming, intra-team review and shared ownership
- 6 Granting
- 7 Revocation
- 8 Membership
- 9 See also
Self-merge[edit | edit source]
Self-merging means +2-ing your own code.
"Self-review is bad for code quality and bad for morale" (Roan).
+2 is for code review, not merging your own stuff[edit | edit source]
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.
Very few changes are trivial enough to self-merge[edit | edit source]
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.
+2 is for things you're qualified to review[edit | edit source]
Social conduct[edit | edit source]
In code review, design discussions, and bug comments, those with +2 power have a special responsibility to see from others' points of view.
Must read for code reviewers[edit | edit source]
- Code review guide
- Security for developers
- Manual:Coding conventions (and subpages)
- Manual:Unit testing (and related pages)
- Database optimization - keep this in mind when making schema changes (which should be implemented following this process)
- Manual:How to debug
[edit | edit source]
If you're working as part of a team, pair programming and 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.
Granting[edit | edit source]
+2 rights on MediaWiki core and extensions are granted to
- Wikimedia Foundation engineers who've passed internal hiring standards (which includes review of previous development work, task assignments, and more), at the discretion of hiring managers
- Community members who have contributed high quality patches, exercised +1 rights well, and demonstrated competence
See Gerrit/Project ownership for a public requests queue.
Revocation[edit | edit source]
+2 rights may be removed if:
- you've repeatedly merged bad code;
- you've repeatedly merged your own code;
- you've failed to socialize high impact changes within the development community;
- you've not followed the guidelines above; or
- you've had an employee agreement / contract which has come to an end, and you have no intent to continue contributing.
Anyone can propose a revocation discussion, any architect in the Wikimedia Foundation (currently Brion Vibber, Tim Starling and Mark Bergsma) can sign off on a revocation for technical or social reasons, and the Wikimedia Foundation's most senior engineering manager (currently Damon Sicore) or the manager's delegate can sign off on a revocation for emergency security matters or obvious policy breaches.
+2 may be reinstated if you demonstrate indicators of improvement.
Membership[edit | edit source]
For a list of people with +2 across mediawiki/*, please see <https://gerrit.wikimedia.org/r/#/admin/groups/11,members>. People who have +2 rights due to membership in the "wmf" LDAP group are not listed (you can get a list of these people by running
ldaplist -l group wmf in WMF labs -
ldaplist -l passwd <uid> for details about each user). There are people outside this list with +2 rights to a specific repository used on WMF sites.