Draft 01 by DrupalCon London

Events happening in the community are now at Drupal community events on www.drupal.org.
You are viewing a wiki page. You are welcome to join the group and then edit it. Be bold!

Given where things are and what is going on, I would like to propose the following:

We have a first draft of the code review process by DrupalCon London. This includes documentation for both applicant and reviewer. Currently, there is some specific discussion happening here. Specifically, this would include the following:

Goals

The mission of reviewing Full Project Applications is to ensure that new contributors have a basic understanding of the Drupal community's core values concerning contributing code to Drupal.org, AND to promote lasting, long-term contributing to the project.

Critical Goals

  • Module duplication: Reduce module duplication and module clutter on Drupal.org as much as possible, without stifling innovation.
  • Licensing: Code/images copied from other sources must be licensed GPLv2 and later.
  • Security: As much as possible catch all security issues in this stage AND educate the developer about them so they won't make more of the same mistake.
  • Drupal API: Determine that applicants have a good overall understanding of Drupal APIs and use them in such a way that their code interacts correctly with Drupal core and other contributed modules.

Other Important Goals

  • Coding Standards and Documentation: Applicants need to be aware of and follow Drupal coding standards and documentation practices.
  • Mentoring: Provide a welcoming environment for new contributors and provide guidance to them on the path of becoming project maintainers.

Specific workflow documentation (tags, issues)

  • Pre-Screening: A reviewer performing the "pre-screening" step will make sure that the application has been submitted correctly and make the appropriate notifications to the applicant. At this stage the issue status should be set to active.
  • Screening: A reviewer performing the "screening" step will check the application for licensing and module duplication issues. Status should remain 'active' until all issues have been resolved. The screener should change the status to 'needs review' when issues have been resolved, or to 'closed won't fix' if issues cannot be resolved.
  • Technical Review: A reviewer performing the "technical review" step will check the code for overall understanding and correct usage of Drupal APIs and for security issues. The reviewer should also note any issues with coding standards and/or documentation. During this phase the status should alternate between 'needs work' and 'needs review'. When all issues have been resolved, the status can be changed to 'reviewed and tested by the community', or to 'closed, won't fix' if issues cannot be resolved.
  • Approval: A git administrator will grant the applicant vetted git user status and mark the application fixed.

Simple documentation for applicants

Project Application Issue Page

What are the current pages for this at the moment?
http://drupal.org/node/1011698

The Project Application Issue Page is where you submit your project applications for review.

  • Create a new issue: Use the project name for the title, set 'Category' to 'task' and 'Status' to 'needs review'.
  • Detailed description: Describe what your project does, including any technical details that you feel will help the reviewer understand your project and how it may differ from other similar projects. Please include the Drupal major version (D6, D7) in your project description.
  • Similar modules: It is important to include the names of other similar modules that may already exist on Drupal.org. Explain how your module is different, and what new features or other innovations it offers. Historically, skipping this step tends to extend the application-to-approval timeframe.
  • Be sure to include a link to your project page.
  • Subscribe Make sure that you subscribe to your issues in the Project Application issue queue, so that you will be notified by email when someone makes a notation on your applicaiton.
  • Responding to issues: If a reviewer raises an issue that requires a modification to your code, whenever possible you should commit patches that address a single issue. Coding standards would be considered a single issue, as would documentation.

Sandbox Project Page

The Sandbox Project Page is where you sell your project to your users.

  • Features and benefits: Here you should describe what your project does for the user, and why they should use it.
  • Other Information: If you have a demo site be sure to link to it here. You may want to include other information like your development roadmap and so forth.

Simple documentation for reviews

What are the current pages for this at the moment?

The Purpose of Documentation

  • Basis For Decision Making: Good documentation provides the git administrator with a reasonable basis to decide whether or not to approve the application.
  • Provides Evidence: Good documentation shows that all of the important points of code review have been considered. It should also show that the applicant has understood and responded appropriately to the issues that have been raised, demonstrating that he/she has the skills to be a project maintainer.

The Project Application Issue Queue

  • Primary Documentation: The application issue queue is the primary place for documenting the code review. In many cases the entire review will be documented there.
  • Not Just For Problems: We tend to think of issue queues as places where we talk about bugs and other problems with the software. For code reviews we also need to provide positive statements when we have completed a part of the review and have not found any problems. When issues have been resolved, we should also state that part of the review has been completed satisfactorily.
  • Note What You Did: Provide some details of what you did. For example, 'Checked the repository for non-GPL files - non found', or 'Searched for similar projects in xyz category...'

The Sandbox Issue Queue

  • Optional:

Process Review Period

We also need to set up a review period. As I said it should be a draft, a living document, and we need to have intervals for more formalized reviews. See this page.

My first reaction is to have a "release" of the process every 6 months (or maybe a year). This means updating documentation on this interval, and making it a more formal procedure to affect the review process.

Comments

On this page?

ccardea's picture

I was just about ready to start marking up this page. Is that what you had in mind?

yeah

zzolo's picture

Yeah, making this a wiki page would be good.

--
zzolo

Your wish is my command

greggles's picture

Your wish is my command (required small db fiddling...sadly it's not something others can do just yet).

Documentation for applicants

ccardea's picture

Not sure what you're looking for here. Are you talking about the project descriptions in the application queue and project page, or are you talking about documentation of the code itself? I know there has been some discussion of having applicants state up front what they did in terms of duplication check. That's actually already stated in the documentation but we probably should include it here.

A first "release" for code reviews.

zzolo's picture

Sorry, I did not articulate myself that well.

So, I would like to think of this as a "release" of the project. Meaning that the handbook pages and "official" documentation around the process are finalized and do not significantly change for X amount of time.

I think this will allow us to iterate through improving the process in a reasonable fashion, as well as making it more efficient to improve as we will have more formalized procedures.

I do realize that there is already documentation and the such, but we have to have a first version to get this new procedure in place.

--
zzolo

I like this idea

ccardea's picture

Most businesses of any size operate on quarterly reviews. That doesn't mean that you have make changes, it's an opportunity to assess the situation, and changes can be more timely if there is a need.

Simple Documentation for Reviews

ccardea's picture

RE: Where are current pages for this? I don't believe there are any. If there are, I haven't found them yet.

Code review for security advisory coverage applications

Group organizers

Group notifications

This group offers an RSS feed. Or subscribe to these personalized, sitewide feeds: