How to review Full Project applications

Last updated February 5, 2012.

• Prerequisites: Suggested knowledge before review.
• Goals: Broad view on how and why to review.
• Workflow: Specific details on how to review.

This guide supports experienced Drupal developers and themers as they review applications to create full projects.

The application issue queue is open and anyone can review issues! We need your help! Even reviewing one application helps a lot!

This is a work in progress; please contact zzolo for discussion.

Prerequisites

Read the following handbook pages before you begin:

• Applying for permission to create full projects
• Writing secure code (and all sub pages)
• Coding standards (and all sub pages)
• Programming best practices (and all sub pages)
• 3rd party libraries on Drupal.org and Why drupal.org doesn't host GPL-"compatible" code.

Goals

Taken from webchick's talk about this process and other experiences of reviewers, the following are the goals and ideas behind the application review methodology.

We have fairly strict licensing requirements for the code that is hosted on drupal.org. It is important to maintain this so that the community of users benefits from all the code, and drupal.org does not have any serious legal problems.

Module duplication is a real problem on drupal.org, and continues to grow. Again, to ensure that the code on drupal.org is useful to the community of people using Drupal, it is important to ensure that module duplication is avoided. Duplication is different from similarity. A similar project that explains how it is different is helpful to the growth of our project.

Security! This should be very obvious. If code is not secure, it means that Drupal's reputation is hurt.

Ensuring that all modules are written to a Drupal standards and best practices empowers all Drupal developers to be able to interact with code (both with developing and understanding) much easier and helps streamlines collaboration.

The code contribution application is often a first step into the Drupal community and should be viewed as a mentoring moment for this person. A good experience here can lead to a much stronger contributor in the long run, and creating a painful process may turn them away from contributing at all. The following should be kept in mind when reviewing:

• Attempt to stay with a single application. Given your time, it is beneficial to stay with a single application (issue ticket) from start to finish. This provides consistency to the applicant and better mechanism for the feedback loop.
• Be supportive. Obviously this is very subjective, but it is important that your comments and review are meant to encourage and foster good code. Positive comments are just as helpful as negative ones; feel free to say how awesome something as well as point out how things need to be improved.
• It is extremely important to provide useful comments. This means not just saying that code is wrong, but explaining why and using links to documentation.
• Keep your expectations at a minimum. Try to see the perspective of the applicant as they may have not ever contributed code to an open source project, may have not read all the Drupal documentation about contributing, may be a brand new developer, or may not even speak English well. This means you may have to do a little hand holding at first, which should be acceptable.
• Even if the code is perfect, you can still offer your experience as a Drupal developer. This application is not just about making sure code is perfect, its about ensuring that we have quality contributors to Drupal. If the code is good enough to accept the application, mark as reviewed, but make suggestions on how the applicant can improve their code to make it better for the community. For instance, suggesting Views integration, looking at X module, or saying how adding a hook here would make this much more extensible, etc.

Workflow

There are two main ways to get full project access:

1. Apply with new module
2. Volunteer to maintain an abandoned module

Drupal.org project application workflow is important to keep tasks organized. Overall, you want to alternate between:

• needs review: the application needs to be reviewed.
• needs work: the application should provide more information to be reviewed.

Application Priority

Application priorities may be implemented to ensure that applications that have been awaiting review for extended periods of time are given priority. Priorities may be set according to priority guidelines by either the applicant or the reviewer and generally apply only to applications with a status of needs review. Applications in which the applicant has failed to respond should not be prioritized. For applications with elevated priority, once a reviewer has continued the review of the application by responding to the applicant according to the project application workflow the application priority should return to normal.

The guidelines on elevating project application priority are as follows:

• Normal: All applications must begin with a normal priority. Applications with elevated priorities should be returned to normal priority once a reviewer has continued the application review.
• Major: Applications with a status of needs review that have been awaiting response from a reviewer for 2+ weeks.
• Critical: Applications with a status of needs review that have been awaiting response from a reviewer for 4+ weeks.

New Module

This route is by far the most time consuming, but also the most important and rewarding. So be thorough but do not arbitrarily create barriers. Throughout these steps, it may be best to have the applicant fix things before further review.

1. Ensure that there is a detailed description of what the module does and a full description of how it differs from any other project.
   • Search for similar modules yourself.
   • Keep in mind that English may be difficult for who is applying.
2. Search through the files uploaded for any possible non-GPL, or third-party library files. Ask the applicant to explain these and make sure to point to relevant documentation.
   • See third party libraries on Drupal.org.
3. Review for security.
   • See Writing secure code (and child pages).
4. Review for best practices.
   • See Programming best practices (and all sub pages).
5. Review for coding standards.
   • See Coding standards (and all sub pages).
   • See also Module documentation guidelines.
6. For a list of important issues that need to be resolved, see Full project applications review, what to expect.

Abandoned Module Applications

The abandoned module process does not actually give the user the Git Vetted permission. Sometimes, someone will want to apply for Full Project access (Git Vetted permission) with the module that they have taken over. Overall, this is fine, but keep the following in mind:

• Get a detailed description of updates. Basically, there needs to be some proof that the user has made enough code changes to be able to review the module.
• The main issue with this circumstance is that there is not really a feasible way to determine how much code was written by the user applying. The assumption that should be made is that if the code reviewed is good enough, and the user has written a significant part of it, then there can be a correlation between their knowledge and quality of code.

Abandoned Applications

For project applications that appear abandoned:

• After ten weeks with a status of needs work: the applicant may be contacted by a reviewer to determine whether the application was indeed abandoned. The action taken by the reviewer should be documented in the project application issue.
• After two weeks following attempted contact: if the applicant has failed to respond the application status may be set to closed (won't fix). The action should be documented and applicant notified that they may reopen the issue if they wish to continue the application process in the future.