[D7] jQuery Nicescroll

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.

Hi el7cosmos,

Here's a simple review of your module, hope it helps you out:

Issues Summary

• (minor) The function "jquery_nicescroll_page_build()" in in the jquery_nicescroll.module file does some complex functions and could do with some in-line commenting.
• (minor) Some of your non-Drupal API code don't use the doxygen commenting (like in https://drupal.org/node/1354#functions)

Basic application checks

• There's a Git repository and project page on your issue list.
• I couldn't find any duplicate projects on the Drupal modules page
• There's only one application in the project applications list.
• I installed your module with the 3.5.1 brach of jQuery Nicescroll and it worked fine :-).

Repository checks

• You do have a Git repository with your code.
• You only have the one 7.x-1.x branch in your repo.
• You have over 5 functions and 900 lines of handwritten code, so its suitable for review

Security review

• With respect to the Drupal code (so not the code in the jQueryNicescroll external code), there are no glaring security issues. Your code also passes the PAReview.sh and Coder reviews.

Licensing checks

• You put the jQuery Nicescroll (which is dual licensed as MIT and GPL licenses) in a library using the Libraries module, which is the ideal solution.
• A visual inspection of your code didn't show any 3rd party code.

Documentation checks

• Given this is a module integrating a 3rd party module into Drupal, the project page is sufficient.
• As above, the README file is sufficient and the instructions work!
• Most of your functions are pretty self-explanatory (like form building and permissions definition) - the function "jquery_nicescroll_page_build()" in in the jquery_nicescroll.module file does some complex functions and could do with some in-line commenting. But this is a minor issue.

Coding standards

• The code passes the PAReview.sh and coder reviews. However, on normal mode there is a warning in jquery-nicescroll.js "File: @file block missing (Drupal Docs) [comment_docblock_file]" but your code does contain the @file block? Don't know exactly what's happening with it.

API and best practices review

• Looking through the code, you seem to follow the API and best practices well.
• To be very pedantic, some of your non-Drupal API code don't use the doxygen commenting (like in https://drupal.org/node/1354#functions) but that is a minor issue.

Overall, a well written module. Good luck with the rest of the review process.
ajosephau

Thanks for the inputs on this module ajosephau.
Settings status appropriately so that the developer is notified.

Hi el7cosmos,

Well done :-), good luck with the rest of your review!

Thanks,
ajosephau

Describe link

Hey el7cosmos,

Nicely done on the module!

Just a few things that I spotted while reading through the code:

1. You may want to allow the user to choose between minified version or the full version of the JQuery Nicescroll library. Better yet, restrict to only the minified version.
2. In the admin.inc form, ln 124, you should use 0 or 1 in the options array rather than TRUE or FALSE, since you are already doing a settype.
3. In the admin.inc form, ln 290, you don't need to wrap check_plain around the t(). If you do '@error' the output will automatically pass into check_plain(). However, I've tested the error message, it is outputting some encoded html so you may just not use check_plain() at all.

That is all, I think your module is good to go!

Your module is really useful! Thanks a lot.

I can see one little helpful improvement: please update your jquery_nicescroll.info with:

configure = admin/config/user-interface/jquery-nicescroll

It will expose the configuration page in the modules list.

+1

I have read this issue, the code and cannot see anything that stands out or hasn't already been addressed.

Please give @el7cosmos git commit access. I have worked with him on the https://drupal.org/project/bootstrap project and it's been a pleasure. He is very attentive to his code.

No problems found, looks like a nice implementation!

Thanks for your contribution, el7cosmos!

I updated your account to let you 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 get 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.

----
Top Shelf Modules - Crafted, Curated, Contributed.