Gerrit/+2

From MediaWiki.org
< Gerrit(Redirected from +2)
Jump to: navigation, search

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.

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

+2 is for things you're qualified to review[edit | edit source]

If you're a very strong JavaScript developer and know about architectural, front-end, performance and user interface considerations for web development, you may be qualified for reviewing almost any front-end change -- but you may not have the background necessary to make a good decision on merging a change to, say, MediaWiki's back-end database schema. Use your own judgment and only merge changesets that you can make a qualified sign-off on.

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]

Very few changes are trivial enough to self-merge[edit | edit source]

Except for documentation fix-ups, don't +2 your own code. "Self-review is bad for code quality and bad for morale." (Roan)

+2-ing your own code is tolerated in some cases where the project only has one maintainer and doesn't contain deployed code. In projects deployed on the Wikimedia cluster, +2-ing your own code is unacceptable and can be grounds for revocation.

Pair programming, intra-team review and shared ownership[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 Erik Möller) 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. There might be people outside this list with +2 rights to a specific repository used on WMF.

See also[edit | edit source]