[D7] Hijri

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.

Hey drpl,

You might want to update the git command as it is using your maintainer command.

Just edit your project, untick maintainer, click show and copy the command.

I will review as soon as I can.

Cheers.

Hi drpl,

Thank you for your efforts, I did some manual review of the code however didn't install it yet. Please find my comments:

Automated Review

Review of the 7.x-1.0 branch (commit efe5340):

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

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

[No: Does not follow] 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

[Yes: Follows] the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

[Yes: Follows] the guidelines for project length and complexity.

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. (*) No need to add empty files. see (hijri.install and hijri.api.php)
2. (*) "variable_get('hijri_display_block', ','),": all variables defined by your module must be deleted in hook_uninstall().
3. (*) hijri_preprocess_node() & hijri_preprocess_comment: doc block is wrong. See https://www.drupal.org/coding-standards/docs#forms
4. (*) More than 2 line space. see line 350-351 of hijri.module.
5. (*) Do not use module_load_include at the initial of file, Add it wherever requires.
6. (*) No need to use switch you can directly pass the value using single variable. get the hijri_display_block value in variable and pass it directly to the function. see line 55 of hijri.module.
7. (*) Unused empty parameter is pass to the function. see line 55, 3rd param of hijri.module.
8. (*) Unused argument passed to t(). see line 16 of hijri.module.
9. (*) Again no need to use switch, Instead you can use ternary operator. see line 280 and 326 of hijri.module.
10. (*) instance witch is created on line 223 need to be deleted in hook_uninstall().

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.

Great, you need to get the review bonus for further review of your module by community members,
see https://www.drupal.org/node/1975228

Removing review bonus tag, you have not listed any reviews in the issue summary? Make sure to read https://www.drupal.org/node/1975228 again. Thanks for helping!

HI,

Please have a look on these points.

1: On settings page i have found the below mentioned notices .
Notice: Undefined property: stdClass::$field_hijri_correction in hijri_preprocess_node() (line 267 of F:\wamp\www\drupal\sites\all\modules\hijri\hijri.module).
Notice: Undefined index: article in hijri_preprocess_node() (line 280 of F:\wamp\www\drupal\sites\all\modules\hijri\hijri.module).
Notice: Undefined property: stdClass::$field_hijri_correction in hijri_preprocess_node() (line 267 of F:\wamp\www\drupal\sites\all\modules\hijri\hijri.module).
Notice: Undefined index: article in hijri_preprocess_node() (line 280 of F:\wamp\www\drupal\sites\all\modules\hijri\hijri.module).
Notice: Undefined property: stdClass::$field_hijri_correction in hijri_preprocess_node() (line 267 of F:\wamp\www\drupal\sites\all\modules\hijri\hijri.module).
Notice: Undefined index: article in hijri_preprocess_node() (line 280 of F:\wamp\www\drupal\sites\all\modules\hijri\hijri.module).
Notice: Undefined property: stdClass::$field_hijri_correction in hijri_preprocess_node() (line 267 of F:\wamp\www\drupal\sites\all\modules\hijri\hijri.module).
Notice: Undefined index: article in hijri_preprocess_node() (line 280 of F:\wamp\www\drupal\sites\all\modules\hijri\hijri.module).
Notice: Undefined property: stdClass::$field_hijri_correction in hijri_preprocess_node() (line 267 of F:\wamp\www\drupal\sites\all\modules\hijri\hijri.module).
Notice: Undefined index: article in hijri_preprocess_node() (line 280 of F:\wamp\www\drupal\sites\all\modules\hijri\hijri.module).
Notice: Undefined property: stdClass::$field_hijri_correction in hijri_preprocess_node() (line 267 of F:\wamp\www\drupal\sites\all\modules\hijri\hijri.module).
Notice: Undefined index: article in hijri_preprocess_node() (line 280 of F:\wamp\www\drupal\sites\all\modules\hijri\hijri.module).
Notice: Undefined property: stdClass::$field_hijri_correction in hijri_preprocess_node() (line 267 of F:\wamp\www\drupal\sites\all\modules\hijri\hijri.module).
Notice: Undefined index: article in hijri_preprocess_node() (line 280 of F:\wamp\www\drupal\sites\all\modules\hijri\hijri.module).
Warning: Invalid argument supplied for foreach() in form_type_checkboxes_value() (line 2337 of F:\wamp\www\drupal\includes\form.inc).

2 : Still i can see a lot of issues on "http://pareview.sh/pareview/httpgitdrupalorgsandboxdrpl2389037git" .Do resolve all the point before making it in needs review mode.

Do resolve all the issues mention in #9 as well as do unit testing before making status needs review.
Don't forget to review other module applications and add those links to the your application to add review bonus.

Thank you for your reviews. Make sure to review projects in the "needs review" state next time because projects in "needs work" are already blocked and need a maintainer response anyway.

Review of the 7.x-1.x branch (commit a13885e):

• Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

1. _hijri_date_block(): do not call theme() here, just return a render array. Drupal core will render it later for you. See https://www.drupal.org/node/930760
2. hijri.test is not really helpful, please fill out this dummy.
3. There are still lots of minor coding standard errors, please fix them.

But otherwise looks RTBC to me. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Assigning to er.pushpinderrana as he might have time to take a final look at this.

Automated Review

Best practice issues identified by pareview.sh / drupalcs / coder. Yes, See coder-results.txt uploaded by Klausi.

Review of the 7.x-1.x branch (commit a13885e):

Manual Review

(+) README.md: The file should be formatted to hard-wrap at 80 characters. See https://www.drupal.org/node/161085#readme

(+) hijri_help(): Help link switch case/path is incorrect. See https://api.drupal.org/api/drupal/modules%21system%21system.api.php/func...

_hijri_date_block(): Klausi also pointed the same thing. Don't use theme() directly, use render arrays. The main reason why the render api exists is it allows for altering by other modules before any html is produced. Also it provides one consistent system to produce any output.

But those issues are not critical application blockers, so...

Thanks for your contribution, Abdullah Bamelhes!

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

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