|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 (merge). This guideline is about the latter.
So, you got +2 rights to MediaWiki core or a WMF-deployed extension/skin.
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 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 Non-Wikimedia extensions
- 4 Social conduct
- 5 Must read for code reviewers
- 6 Pair programming, intra-team review and shared ownership
- 7 Granting
- 8 Revocation
- 9 Membership
- 10 See also
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
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
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 can generally be self-merged as long as the commit it is reverting was recent.
+2 is for things you're qualified to review
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.
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
- 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
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.
+2 rights on MediaWiki core and extensions and skins 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; and
- Community members who have contributed high quality patches, exercised +1 rights well, and demonstrated competence.
Gerrit/Project ownership describes the rights and groups structure in gerrit and how to request +2 access on a project. A "Project owner" is equivalent to someone with +2 rights on that project.
+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, the Wikimedia Technical Committee can sign off on a revocation for technical or social reasons, and anyone authorized by Wikimedia Foundation's Board of Trustees (e.g. WMF's Director of Technical Operations) can sign off on a revocation for emergency security matters or obvious policy breaches.
+2 may be reinstated if you demonstrate indicators of improvement.
For a list of people with +2 rights across mediawiki/*, see https://gerrit.wikimedia.org/r/#/admin/groups/11,members and the wmf LDAP group. There are people outside this list with +2 rights to specific repositories used on Wikimedia sites.