Last updated 7 March 2017.
This guide supports Drupal developers and themers as they review project applications. It is the flip side of Apply for permission to opt into security advisory coverage.
Application reviews are open to everyone and a great way to support the Drupal community! Please start with issues that have a review bonus or with issues that did not get a review in a long time. We need your help! Even reviewing one application helps a lot!
The following are the goals and ideas behind the application review methodology:
Enabling new contributors is the ultimate goal of this process. 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. It is important for reviewers to help recommend avenues for new contributors to get involved, i.e. with similar projects, ongoing issues, active workgroups, etc. Good mentors help new contributors more with their knowledge of community practices than with code style changes. The following should be kept in mind when reviewing:
- 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 is, 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.
- 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.
- 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.
Security and code quality (high priority) is very important to Drupal's reputation. So, ensuring that all projects are written to Drupal's coding standards and best practices is a critical role for reviewers. Writing high quality, secure code empowers all Drupal users and developers to be able to interact with it, both with understanding and developing, and helps streamlines collaboration.
Licensing (high priority) requirements for the code that is hosted on drupal.org is fairly strict. 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. 3rd party libraries on Drupal.org have specific requirements which extends to Why drupal.org doesn't host GPL-"compatible" code.
Module duplication (lower priority) makes it hard to find the right module, which 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. However, duplication is different from similarity. A similar project that explains how it is different is helpful to the growth of our project. The Similar Module Review Workgroup helps with finding duplicate modules, analyzing their similarities/differences, and seeing if there is room for collaboration between them.
There are two main ways to get access to mark projects as covered by security advisories:
- Apply with new project
- Volunteer to maintain an unsupported project
Drupal.org project application workflow is important to keep tasks organized. Overall, you want to alternate status between:
- needs review: the application needs to be reviewed.
- needs work: the application should provide more information to be reviewed.
The project application workflow page describes when to change the status.
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.
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.
- Ensure that there is a detailed description of what the project does and a full description of how it differs from any other project.
- Search for similar projects yourself.
- Keep in mind that English may be difficult for who is applying.
- 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.
- Review for security.
- See Writing secure code (and child pages). Applications that have security issues should be tagged with 'PAReview: security'.
- Review for best practices.
- See Programming best practices (and all sub pages).
- Review for coding standards.
- For a list of important issues that need to be resolved, see Review process for security advisory coverage: what to expect.
The unsupported project process does not actually give the user the Git Vetted permission. Sometimes, someone will want to apply for the Git vetted role with the project 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 project.
- 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.
After four weeks with a status of needs work: the PAReview robot will close the application with a note saying that it appears abandoned, but that the applicant may reopen the issue if they wish to continue the application process in the future.