PAReview: Review Template

You are viewing a wiki page. You are welcome to join the group and then edit it. Be bold!

Do not simply copy/paste this template. Walk through the items and update them as needed: choose from yes/no, list found issues. In general, all text within the square brackets is for the reviewer only and should be updated by the review results. To indicate particular points use these more descriptive templates.
Note: if you are doing a review to earn a review bonus, you need to read through the source code of the project and list all identified issues in corresponding sections of this template (mostly these are 'Secure code' and 'Coding style & Drupal API usage').

Automated Review

[Best practice issues identified by pareview.sh / drupalcs / coder. Please don't copy/paste all of the results unless they are short. If there are a lot, then post a link to the automated review and mention that problems should be addressed.]

Note that perfect adherence to Drupal Coding Standard is NOT a reason to block an application, except for total disregard of them. However, modules should follow them as closely as possible.

Manual Review

Individual user account
[Yes: Follows / No: Does not follow] the guidelines for individual user accounts.
No duplication
[Yes: Does not cause / No: Causes] module duplication and/or fragmentation.
Master Branch
[Yes: Follows / No: Does not follow] the guidelines for master branch.
Licensing
[Yes: Follows / No: Does not follow] the licensing requirements.
3rd party assets/code
[Yes: Follows / No: Does not follow] the guidelines for 3rd party assets/code.
README.txt/README.md
[Yes: Follows / No: Does not follow] the guidelines for in-project documentation and/or the README Template.
Code long/complex enough for review
[Yes: Follows / No: Does not follow] the guidelines for project length and complexity.
Secure code
[Yes: Meets the security requirements. / No: List of security issues identified.]
Coding style & Drupal API usage
[List of identified issues in no particular order. Use (*) and (+) to indicate an issue importance. Replace the text below by the issues themselves:
  1. (*) Major finding, needs work
  2. (+) Release blocker
  3. Just a recommendation
  4. ...]

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.

<h3>Automated Review</h3>
<p>[Best practice issues identified by <a href="http://pareview.sh/">pareview.sh</a> / drupalcs / coder. Please don't copy/paste all of the results unless they are short. If there are a lot, then post a link to the automated review and mention that problems should be addressed.]</p>

<p><em>Note that perfect adherence to <a href='https://www.drupal.org/coding-standards'>Drupal Coding Standard</a> is <b>NOT</b> a reason to block an application, except for total disregard of them.  However, modules should follow them as closely as possible.</em></p>

<h3>Manual Review</h3>
<dl>
  <dt>Individual user account</dt>
  <dd>[Yes: Follows / No: Does not follow] the <a href="https://www.drupal.org/node/2765201">guidelines for individual user accounts</a>.</dd>

  <dt>No duplication</dt>
  <dd>[Yes: Does not cause / No: Causes] module <a href="https://www.drupal.org/node/23789">duplication and/or fragmentation</a>.</dd>

  <dt>Master Branch</dt>
  <dd>[Yes: Follows / No: Does not follow] the <a href="https://www.drupal.org/node/1127732">guidelines for master branch</a>.</dd>

  <dt>Licensing</dt>
  <dd>[Yes: Follows / No: Does not follow] the <a href="https://www.drupal.org/licensing/faq">licensing requirements</a>.</dd>

  <dt>3rd party assets/code</dt>
  <dd>[Yes: Follows / No: Does not follow] the <a href="https://www.drupal.org/node/422996">guidelines for 3rd party assets/code</a>.</dd>

  <dt>README.txt/README.md</dt>
  <dd>[Yes: Follows / No: Does not follow] the <a href="https://www.drupal.org/node/447604">guidelines for in-project documentation</a> and/or the <a href="https://www.drupal.org/node/2181737">README Template</a>.</dd>

  <dt>Code long/complex enough for review</dt>
  <dd>[Yes: Follows / No: Does not follow] the <a href="https://groups.drupal.org/node/195848">guidelines for project length</a> and complexity.</dd>

  <dt>Secure code</dt>
  <dd>[Yes: Meets the <a href="https://www.drupal.org/writing-secure-code">security requirements</a>. / No: List of security issues identified.]</dd>

  <dt>Coding style &amp; Drupal API usage</dt>
  <dd>[List of identified issues in no particular order. Use (*) and (+) to indicate an issue importance. Replace the text below by the issues themselves:
    <ol>
      <li>(*) Major finding, needs work</li>
      <li>(+) Release blocker</li>
      <li>Just a recommendation</li>
      <li>...]</li>
    </ol>

    <p>The starred items (*) are fairly big issues and warrant going back to <strong>Needs Work</strong>. Items marked with a plus sign (+) are important and should be addressed before a <strong>stable project release</strong>. The rest of the comments in the code walkthrough are recommendations.</p>
  </dd>
</dl>

<p>If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.</p>

<p><i>This review uses the <a href="https://groups.drupal.org/node/427683">Project Application Review Template</a>.</i></p>

Comments

I have edited a few pages in

mpdonadio's picture

I have edited a few pages in https://drupal.org/node/636570 to point to this page.

What does Individual user account means??

madhusudanmca's picture

Hi mpdonadio,

What does Individual user account actually means?? Because whenever any user is submitting any application for review, he will ultimately have his/her individual account. I don't think that any anonymous user can post any project here :)

I am asking this, because while reviewing one application I saw one project that seems to be posted by some organizational name and all the code was committed by some other user. Please see for details.

Thanks!!

Organization accounts are

klausi's picture

Organization accounts are somewhat fine (see https://www.drupal.org/node/1863498 ), but they cannot apply for full project access, we only grant that to individual developers. Whenever we spot an application submitted by an organization we just switch it to the individual developer, I did that in the mentioned issue.

Thanks for the clarification

madhusudanmca's picture

Thanks for the clarification klausi!!

Reverting spam.

mpdonadio's picture

Reverting spam.

Removing Duplicate from a blockers ?

darol100's picture

Should we remove the section duplicate module from this template ?

As Klausi mention on this comment

As we change our project application process in #2453587: [policy, no patch] Changes to the project application review process we are not allowed to block applications base on module duplication. We can still recommend collaboration over competition, but we should not enforce it.

No, this should still stay.

mpdonadio's picture

No, this should still stay. We still want to discourage module duplication, but we will no longer block applications for it. What probably needs to change is the language on https://groups.drupal.org/node/184389#duplicate to reflect the new policy.

Guidelines for individual user page not found

patilvishalvs's picture

Receiving Page not found(404) for guidelines for individual user accounts(https://www.drupal.org/node/272587)

Code Review for opt in security advisory coverage | Home

Group organizers

Group notifications

This group offers an RSS feed. Or subscribe to these personalized, sitewide feeds: