Increasing efficiency in manual code reviews

Quicklinks

• PAReview: Review Template
• PAReview: duplication
• PAReview: README.txt
• PAReview: Master Branch
• PAReview: major coding standards
• PAReview: License
• PAReview: access administration vs. administer site configuration
• PAReview: files[] without classes or interfaces
• PAReview: 3rd party code
• PAReview: Individual user account
• PAReview: Multiple Applications
• PAReview: Already Approved
• PAReview: Idle Application
• PAReview: Code too short
• PAReview: Code too simple
• PAReview: Promoting someone

Templates

PAReview: duplication

This sounds like a feature that should live in the existing PROJECT_NAME project. Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition. Please open an issue in the PROJECT_NAME issue queue to discuss what you need. You should also get in contact with the maintainer(s) to offer your help to move the project forward. If you cannot reach the maintainer(s) please follow the abandoned project process.

If that fails for whatever reason please get back to us and set this back to "needs review".

<dl>
<dt>Duplication</dt>
<dd>This sounds like a feature that should live in the existing PROJECT_NAME project. Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition. Please open an issue in the PROJECT_NAME issue queue to discuss what you need. You should also get in contact with the maintainer(s) to offer your help to move the project forward. If you cannot reach the maintainer(s) please follow the <a href="https://www.drupal.org/node/251466">abandoned project process</a>.

If that fails for whatever reason please get back to us and set this back to "needs review".</dd>
</dl>

PAReview: README.txt

Please take a moment to make your README.txt follow the guidelines for in-project documentation.

<dl>
<dt>README.txt</dt>
<dd>Please take a moment to make your README.txt follow the <a href="https://www.drupal.org/node/447604">guidelines for in-project documentation</a>.</dd>
</dl>

PAReview: project page:

Please take a moment to make your project page follow tips for a great project page.

<dl>
<dt>project page</dt>
<dd>Please take a moment to make your project page follow <a href="https://www.drupal.org/node/997024">tips for a great project page</a>.</dd>
</dl>

PAReview: Master Branch

It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.

<dl>
<dt>Master Branch</dt>
<dd>It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is <a href="https://www.drupal.org/node/1127732">Moving from a master branch to a version branch.</a> For additional resources please see the documentation about <a href="https://www.drupal.org/node/1015226">release naming conventions</a> and <a href="https://www.drupal.org/node/1066342">creating a branch in git</a>.</dd>
</dl>

PAReview: major coding standards / best practice issues by pareview.sh / drupalcs

An automated review of your project has found some issues with your code; As coding standards make sure projects are coded in a consistent style we ask you to please have a look at the report and try to fix them. However, please note that some of the issues found are possibly false positives and fixing all issues is not a requirement for getting through the application process.

You can find the results of the automated report at LINK.
OR
I´ve attached the automated report as text file to this comment.

Submit the project for review at http://pareview.sh/ (you can also install pareview.sh and drupal code sniffer locally instead) and provide a link to the results (or post as an attachment) in the application thread. Please do not paste the full results into the issue, as giant ‘wall-of-wrong’ posts are extremely demotivating to applicants.

<dl>
<dt>Major coding standards / best practice issues</dt>
<dd>An automated review of your project has found some issues with your code; As coding standards make sure projects are coded in a consistent style we ask you to please have a look at the report and try to fix them. However, please note that some of the issues found are possibly false positives and fixing all issues is not a requirement for getting through the application process.

You can find the results of the automated report at LINK.
OR
I´ve attached the automated report as text file to this comment.</dd>
</dl>

PAReview: License

Please remove the LICENSE.txt file. Drupal will add the appropriate version automatically during packaging so your repository should not include it.

<dl>
<dt>License</dt>
<dd>Please remove the LICENSE.txt file. Drupal will add the appropriate version automatically during packaging so your repository should not include it.</dd>
</dl>

PAReview: access administration vs. administer site configuration

The administration menu callback should probably use "administer site configuration" - which implies the user can change something - rather than "access administration pages" which is about viewing but not changing configurations.

<dl>
<dt>access administration vs. administer site configuration</dt>
<dd>The administration menu callback should probably use "administer site configuration" - which implies the user can change something - rather than "access administration pages" which is about viewing but not changing configurations.</dd>
</dl>

PAReview: files[] without classes or interfaces

*start by copying the files that do not contain classes*
Those lines should be removed. It's only necessary to declare files[] if they declare a class or interface.

Please note: The Coder module currently has an unresolved flaw which will prompt you to add file declarations to your .info file even when it's not necessary to do so. Please do not try to make this warning go away by declaring files which do not contain classes or interfaces.

<dl>
<dt>files[] without classes or interfaces</dt>
<dd>*start by copying the files that do not contain classes*
Those lines should be removed. It's only necessary to <a href="https://www.drupal.org/node/542202#files">declare files[] if they declare a class or interface</a>.

Please note: The Coder module currently has an <a href="https://www.drupal.org/node/1284150">unresolved flaw</a> which will prompt you to add file declarations to your .info file even when it's not necessary to do so. Please do not try to make this warning go away by declaring files which do <strong>not</strong> contain classes or interfaces.</dd>
</dl>

PAReview: 3rd party code

*name of code* appears to be 3rd party code. 3rd party code is not generally allowed on Drupal.org and should be deleted. This policy is described in the getting involved handbook. It also appears in the terms and conditions you agreed to when you signed up for Git access, which you may want to re-read, to be sure you're not violating other terms.

The Libraries API module is a recommended method for adding 3rd party dependencies without directly including the code on Drupal.org.

<dl>
<dt>PAReview: 3rd party code</dt>
<dd>*name of code* appears to be 3rd party code. 3rd party code is not generally allowed on Drupal.org and should be deleted. This policy is described in the <a href="https://www.drupal.org/node/422996">getting involved handbook</a>. It also appears in the <a href="https://www.drupal.org/node/1001544">terms and conditions</a> you agreed to when you signed up for Git access, which you may want to re-read, to be sure you're not violating other terms.

The <a href="https://www.drupal.org/project/libraries">Libraries API module</a> is a recommended method for adding 3rd party dependencies without directly including the code on Drupal.org.</dd>
</dl>

PAReview: Non-individual user account

It seems you are using a non-individual account.
Please note that organization accounts cannot be approved for Git commit access. See Drupal.org Terms of Service / Accounts for details on what is allowed. Please update your user profile so we don't have to assume this is an account shared by more users.

<dl>
<dt><a name="account"></a>PAReview: Non-individual user account</dt>
<dd>It seems you are using a non-individual account.<br />
Please note that organization accounts cannot be approved for Git commit access. See <a href="https://www.drupal.org/terms#accounts">Drupal.org Terms of Service / Accounts</a>&nbsp;for details on what is allowed. Please update your user profile so we don't have to assume this is an account shared by more users.
</dd>
</dl>

PAReview: Multiple Applications

It appears there are multiple project applications opened under your username:

Project 1: Application link
Project 2: Application link

Since a successful completion of the project application process results in the applicant being granted the vetted role, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers, which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as closed(duplicate), with only one application left open.

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the application left open as a duplicate, and re-open one of the project applications which had been closed.

<dl>
<dt><a name="multiple"></a>PAReview: Multiple Applications</dt>
<dd>It appears there are multiple project applications opened under your username:

Project 1: Application link
Project 2: Application link

Since a successful completion of the project application process results in the applicant being granted the <em>vetted</em> role, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers, which in turn results in longer wait times for everyone in the queue.  With this in mind, your secondary applications have been marked as <em>closed(duplicate)</em>, with only one application left open.

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the application left open as a duplicate, and re-open one of the project applications which had been closed.
</dl>
</dd>

PAReview: Already Approved

It appears that you have successfully completed the application process with a different application, and been granted the vetted role.

Project: APPLICATION_LINK

Once the first application has been successfully approved, an applicant can opt into security coverage without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers, which in turn results in longer wait times for everyone in the queue.

Instead, you can post to the Peer Review group for feedback, here:

https://groups.drupal.org/peer-review/about

With this in mind, I have marked your application as Closed (duplicate). If this is incorrect, and you don't yet have the ability to opt into security coverage, then please feel free to re-open this application.

<dl>
<dt>Already Approved</dt>
<dd>It appears that you have successfully completed the application process with a different application, and been granted the <em>vetted</em> role.

Project:  APPLICATION_LINK

Once the first application has been successfully approved, an applicant can opt into security coverage without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers, which in turn results in longer wait times for everyone in the queue.

Instead, you can post to the Peer Review group for feedback, here:

https://groups.drupal.org/peer-review/about

With this in mind, I have marked your application as <em> Closed (duplicate)</em>.  If this is incorrect, and you don't yet have the ability to opt into security coverage, then please feel free to re-open this application.
</dl>
</dd>

PAReview: Idle Application

This thread has been idle, in the 'needs work' state with no activity for several months. Therefore, I am assuming that you are no longer pursuing this application, and marking it as closed (won't fix).

If this is incorrect, and you are still pursuing this application, then please feel free to re-open the thread and set the issue status to "needs work" or "needs review", depending on the current status of your code.

<dl>
  <dt>Idle Application</dt>
  <dd>This thread has been idle, in the 'needs work' state with no activity for several months. Therefore, I am assuming that you are no longer pursuing this application, and marking it as closed (won't fix).

If this is incorrect, and you are still pursuing this application, then please feel free to re-open the thread and set the issue status to "needs work" or "needs review", depending on the current status of your code.</dd>
</dl>

PAReview: Code too short

This project is too short to approve you as git vetted user. We are currently discussing how much code we need, but everything with less than 120 lines of code or less than 5 functions cannot be seriously reviewed. However, we can promote this single project manually to a full project for you.

<dl>
  <dt>Code too short</dt>
  <dd>This project is too short to approve you as git vetted user. <a href="https://groups.drupal.org/node/195848">We are currently discussing how much code we need</a>, but everything with less than 120 lines of code or less than 5 functions cannot be seriously reviewed. However, we can promote this single project manually to a full project for you.</dd>
</dl>

PAReview: Code too simple

This project does not demonstrate sufficient hook usage and/or API integration, therefore it is only eligible for a one-time promotion to a full project. You can find more info at Working with the Drupal API.

This is an important criterion so that code integrates well and can be improved over time. I encourage you to continue developing and gaining from the feedback available in the git approval process.

Thank you for you contributions and understanding.

<dl>
  <dt>Code too simple</dt>
  <dd>This project does not demonstrate sufficient hook usage and/or API integration, therefore it is only eligible for a one-time promotion to a full project.  You can find more info at <a href="/developing/api">Working with the Drupal API</a>.

This is an important criterion so that code integrates well and can be improved over time.  I encourage you to continue developing and gaining from the feedback available in the git approval process.

Thank you for you contributions and understanding.</dd>
  </dl>

PAReview: Promoting someone

Thanks for your contribution, PERSONNAME!

I updated your account so you can opt into security advisory coverage now.

Here are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

You can find lots more contributors chatting on Slack or IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

Thanks for your contribution, PERSONNAME!

I updated your account so you can opt into security advisory coverage now.

<p>Here are some recommended readings to help with excellent maintainership:</p>
<ul>
  <li><a href="/user/1">Dries</a>' post on <a href="http://buytaert.net/responsible-maintainers">Responsible maintainers</a></li>
  <li><a href="https://www.drupal.org/node/7765">Best practices for creating and maintaining projects</a></li>
  <li><a href="https://www.drupal.org/node/711070">Maintaining a drupal.org project with Git</a></li>
  <li><a href="https://www.drupal.org/node/52287">Commit messages - providing history and credit</a></li>
  <li><a href="https://www.drupal.org/node/1015226">Release naming conventions</a>.</li>
  <li><a href="https://www.drupal.org/node/383956">Helping maintainers in the issue queues</a></li>
</ul>

You can find lots more contributors chatting on <a href="/slack">Slack</a> or <a href="/irc">IRC</a> in #drupal-contribute. So, come hang out and <a href="/getting-involved">stay involved</a>!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are <a href="/project/issues/projectapplications?status=8">pending review</a>. I encourage you to learn more about that process and join the <a href="https://groups.drupal.org/code-review">group of reviewers</a>.

Thanks to the dedicated reviewer(s) as well.