The name of your module or theme:
Biblio DAI (biblio_dai)

A detailed description of what it does:
This module adds a Digital Author Identifier (DAI) field to the contributor data within the Bibliography module (biblio). The DAI is a unique national number for every author active within a Dutch university, university of applied sciences, or research institute.

A link to its sandbox project on Drupal.org:
https://www.drupal.org/sandbox/gkaas/2495045

Git clone command:
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/gkaas/2495045.git

The intended Drupal core version (eg. 6.x or 7.x):
7.x

Module review by Sutharsan:
https://www.drupal.org/node/2513450

CommentFileSizeAuthor
#6 Biblio_DAI.png73.69 KBSamirMtl
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

gkaas’s picture

Issue summary: View changes
PA robot’s picture

Status: Needs review » Needs work

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxgkaas2495045git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

rrfegade’s picture

Hi Gerben Kaas,

Please see the below mentioned errors from your module and rectify them.

Automated Review

There are few errors reported by the automated code review tool, Please correct them first, here is the report : http://pareview.sh/pareview/httpgitdrupalorgsandboxgkaas2495045git

Manual Review

Individual user account
Yes: Follows the guidelines for individual user accounts.
No duplication
Yes: Does not cause module duplication and/or fragmentation.
Master Branch
Yes: Follows the guidelines for master branch.
Licensing
Yes: Follows the licensing requirements.
3rd party assets/code
Yes: Follows the guidelines for 3rd party assets/code.
README.txt/README.md
No: Does not follow the guidelines for in-project documentation and/or the README Template.
Code long/complex enough for review
No: Does not Follows the guidelines for project length and complexity. Need approx. 5 functions and 120 lines of code, How much code do we need to approve a user?
Secure code
Yes: Meets the security requirements.
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. (*) Doesn't follow README guidelines
  2. (*) Module is short
  3. Please resolve the codesniffer issues
  4. All constants defined by a module must be prefixed with the module's name, expected "BIBLIO_DAI_CID_INDEX" but found "CID_INDEX"
    expected "BIBLIO_DAI_DAI_MIN_CHARS" but found "DAI_MIN_CHARS"
    expected "BIBLIO_DAI_DAI_MAX_CHARS" but found "DAI_MAX_CHARS"
  5. Format should be "* Implements hook_foo().", "* Implements
    hook_foo_BAR_ID_bar() for xyz_bar().",, "* Implements
    hook_foo_BAR_ID_bar() for xyz-bar.html.twig.", or "* Implements
    hook_foo_BAR_ID_bar() for xyz-bar.tpl.php.".

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.

This review uses the Project Application Review Template.

gkaas’s picture

Status: Needs work » Needs review

Hi Ravindra Fegade,

Thanks, I have resolved most issues. I agree that the module is quite short, but I guess there's not much to add at this moment...

gkaas’s picture

Issue summary: View changes

The module is updated after a review by Sutharsan:
https://www.drupal.org/node/2513450

SamirMtl’s picture

Issue summary: View changes
FileSize
73.69 KB

Hi gkaas
By enabling biblio_dai module, (fresh instalation, no already content) i have this notice error : (Please see the screenshot)
Notice: Undefined variable: rows in biblio_dai_author_dai_view() (line 115 of /home/samir/projects/drupal-review/target/drupal/sites/all/modules/biblio_dai/biblio_dai.module).
May be, it is better to check if there are contributor into the result (biblio_dai.module line 103) before you seed the table rows and you can add
'empty' =>t('Your table is empty') in : $output .= theme('table', array('header' => $header, 'rows' => $rows,'empty' =>t('Your table is empty'),)); (biblio_dai.module line 115)

SamirMtl’s picture

Status: Needs review » Needs work
gkaas’s picture

Thanks for your review Samir! I'll fix this...

gkaas’s picture

Status: Needs work » Needs review

Fixed the issue as reported by Samir in comment #6...

hesnvabr’s picture

/var/www/html/d7/2495045 (7.x-1.x=) $ cat biblio_dai.module

// Output results as table.
$output .= theme('table', array(
'header' => $header,
'rows' => $rows,
'empty' => t('The author table is empty.'),
));
return $output;
}
Expected 4 spaces but i found 8 in array .
-> header
-> rows
-> empty
-> ))

gkaas’s picture

Thanks Pranav, I have fixed an issue and hopefully this one too.

Warped’s picture

Status: Needs review » Postponed (maintainer needs more info)

Thank you for your contribution!

After 2017 March 7 everyone can promote a project to a full project.  A full project has a short project name and a drupal.org/project URL.  It can also have releases (like alpha1 or 1.0).  Edit your sandbox project, and then choose the 'Promote' tab.

https://www.drupal.org/docs/8/understanding-drupal-version-numbers/drupa...
https://www.drupal.org/docs/8/choosing-a-drupal-version/what-do-version-...
https://www.drupal.org/docs/8/understanding-drupal-version-numbers/what-...
https://www.drupal.org/docs/8/choosing-a-drupal-version/release-stable-v...

If you'd like to opt into security coverage, please ensure your module is ready for a full release, and then set this issue back to 'needs' review

Immense apologies for how long it took to get to this review completed.

apaderno’s picture

Status: Postponed (maintainer needs more info) » Closed (won't fix)

I am closing this application for the lack for replies. I take the OP just needed to be able to promote the project.