← Back to team overview

launchpad-dev team mailing list archive

Launchpad Community Code Reviewers

 

Hello,

Anyone who has ever contributed code to Launchpad knows that we take peer code reviews very seriously.  Almost all Canonical employees on the team are code reviewers and most spend one day a week doing nothing but reviews.  It is a huge commitment of resources but one that we think is very well worth it.

In recognition of the many contributions of community contributors we are opening up the review process to community contributors who have demonstrated an understanding of the project and have contributed significantly.  As with Canonical employees, individuals will be selected by the team leads in consultation with me in my role as the Review team lead.

For historical reasons I think we can blame on Aaron Bentley, new Launchpad reviewers are known as 'mentats' while they are in their mentoring phase.   Each mentat is paired with one mentor, preferably someone in a close time zone, who will guide them through the reviewing process, ensure the reviews done are thorough, strike an appropriate tone, and are a learning experience for both parties.  We strive for our reviews to be a conversation about the code with the intent of sharing knowledge as well as enforcing standards, encouraging good design, and preventing errors.  Reviews by mentats are not sufficient for landing until the review is approved by the mentor.  The mentoring process lasts as long as the mentor thinks is necessary but it is at least one development cycle.

The nature of Launchpad as a hosted service necessitates that we impose an extra layer of review on community reviewers.  Launchpad and Canonical are entrusted by all Launchpad users with private data we have an obligation to keep safe.  Since our build system automatically deploys new code to our edge servers with no intervention after it lands in the code base.  That code then has access to all of the Launchpad database.  In consultation with James Troup in his role as Canonical's security tsar we have concluded that all code must be vetted by a Canonical employee before having access to production data.  Should we ever make a mistake that compromises that data we would suffer a tremendous blow to our reputation and possible legal repercussions.  Were such a breach to occur due to an honest mistake or malice by a community contributor, with no employee oversight, we would have a much more difficult time explaining the situation to our customers and could conceivably be at greater exposure. 

For community reviewers that means even after graduation, reviews of community contributed code will still need sign off by a Canonical employee.  Reviews by a community reviewer of branches submitted by Canonical employees can skip the extra step.  The Canonical employee who signs off on the review will also be responsible for landing those changes through PQM.

For the reasons outlined above we are also unable to allow community contributors to submit code directly to PQM.

We have discussed the possibility of allowing code to be contributed to a separate repository that is not automatically deployed but such a scheme has not yet been designed and is not planned for the near term.

The new community reviewer program is designed to recognize and reward the people[1] who help to make Launchpad great.  We value your insights and hard work and want to give you the chance to do even more.

Best,

Brad Crittenden

[1] https://dev.launchpad.net/Contributions

Attachment: PGP.sig
Description: This is a digitally signed message part