Increasing efficiency in manual code reviews

I thought these ideas might be useful to other manual reviewers AND provide templates for what to say in the automated reviews.

I setup the mywords plugin with about a dozen different phrases. Then I can be nice to newcomers while providing lots of detail.

We should tweak these to be as friendly, informative, mentor-focused, and brief as possible. I know some of those are at odds with each other :)

The <dl> tags are present for reference, to show that you should be adding them around your entire post to make the definition list work properly. You would include all of your <dt> <dd> pairs between the <dl> tags.

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="http://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="http://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="http://drupal.org/node/1127732">Moving from a master branch to a version branch.</a> For additional resources please see the documentation about <a href="http://drupal.org/node/1015226">release naming conventions</a> and <a href="http://drupal.org/node/1066342">creating a branch in git</a>.</dd>
</dl>

PAReview: coder on minor

I noticed some very small code style issues. Please run the Coder module on "minor" setting to help catch these. The coding standards have even more information in this area. I noticed things like ...

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>coder on minor</dt>
<dd>I noticed some very small code style issues. Please run the <a href="http://drupal.org/project/coder">Coder module</a> on "minor" setting to help catch these. The <a href="http://drupal.org/coding-standards">coding standards</a> have even more information in this area. I noticed things like ...

Please note: The Coder module currently has an <a href="http://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: 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="http://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="http://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: Licensing issues

*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>Licensing issues</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="http://drupal.org/node/422996">getting involved handbook</a>. It also appears in the <a href="http://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="http://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: Multiple Applications

It appears that there have been multiple project applications opened under your username:

Project 1: Application link
Project 2: Application link

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, 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 (chosen at random).

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 'open' application as a duplicate, and re-open one of the project applications which had been closed.

<dl>
<dt>Multiple Applications</dt>
<dd>It appears that there have been multiple project applications opened under your username:

Project 1: Application link
Project 2: Application link

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, 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 (chosen at random).

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 'open' application as a duplicate, and re-open one of the project applications which had been closed.</dd>
</dl>

PAReview: Already Approved

It appears that you have successfully completed the application process with a different application, and been granted the 'create full projects' permission:

Project: Application Link

Once their first application has been successfully approved, then an 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, I have marked your application as 'closed(duplicate)'. If this is incorrect, and you do not yet have the ability to create full projects, 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 'create full projects' permission:

Project:  Application Link

Once their first application has been successfully approved, then an 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, I have marked your application as 'closed(duplicate)'.  If this is incorrect, and you do not yet have the ability to create full projects, then please feel free to re-open this application.</dd>
</dl>

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 persuing 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 thred 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 persuing 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 thred 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.

<dl>
<dt>Code too short</dt>
<dd>This project is too short to approve you as git vetted user. <a href="http://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.</dd>
</dl>

PAReview: Promoting someone

Thanks for your contribution, PERSONSNAME! Welcome to the community of project contributors on drupal.org.

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

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.

As you continue to work on your module, keep in mind: Commit messages - providing history and credit and Release naming conventions.

Thanks to the dedicated reviewer(s) as well.

Thanks for your contribution, PERSONSNAME! Welcome to the community of project contributors on drupal.org.

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

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="http://drupal.org/project/issues/projectapplications?status=8">pending review</a>. I encourage you to learn more about that process and join the <a href="http://groups.drupal.org/code-review">group of reviewers</a>.

As you continue to work on your module, keep in mind: <a href="/node/52287">Commit messages - providing history and credit</a> and <a href="/node/1015226">Release naming conventions</a>.

Thanks to the dedicated reviewer(s) as well.