How to review security advisory coverage applications

Last updated on
22 September 2023

Before reviewing applications, please see Tools to use for reviews.

This guide is about to review security advisory coverage 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.

Goals

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 work groups, 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 Security advisory coverage application checklist is a good guide for reviewing an application and the PAReview: Review Template is a good template to utilize while reviewing applications.

Workflow

Security advisory coverage 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 Security advisory coverage application workflow page describes when to change the status.

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 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 applies
  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.
  3. Review for security.
    • See Writing secure code (and child pages). Applications that have security issues should be tagged with PAreview: security.
  4. Review for best practices.
  5. Review for coding standards.
  6. For a list of important issues that need to be resolved, see Review process for security advisory coverage: what to expect.

Help improve this page

Page status: No known problems

You can: