[D7] Views Ajax Overlay

Git clone failed for http://git.drupal.org/sandbox/etroid/2493667.git while invoking http://pareview.sh/pareview/httpgitdrupalorgsandboxetroid2493667git

Git clone failed. Aborting.

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 Martin Keereman,

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

Automated Review

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

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 too short

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.

http://pareview.sh/pareview
There are two small things you can still do from http://pareview.sh/pareview/httpgitdrupalorgsandboxetroid2493667git

• /js/views_ajax_overlay.js: move the "else if {" to a new line
• /README.txt: For the line length error (if it matters), maybe it is better just to mention the class ".view-ajax-overlay .overlay" that needs to be altered. If somebody is familiar with CSS they can look in your stylesheet.

The Views related errors are fine because that's the coding standard used there.

Similarity with other modules
Views Ajax Overlay works with exposed filters whereas Views Ajax Fade does not.

The main difference (for me) between this module and Views Sexy Throbber is that it allows a more programmatic approach versus VST's back-end based configuration. The per-View setting provided here could also be more useful in some cases.

However, have you investigated trying to add the functionality in this module to Views Sexy Throbber? It can be argued that the overlap is large enough that it might be preferable to get your new features into that existing module.

Hi Etroid,

Add your git clone command in project issue summary.
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/Etroid/2493667.git views_ajax_overlay

Some errors reported by the automated code review tool, you need to fix that error.
http://pareview.sh/pareview/httpgitdrupalorgsandboxetroid2493667git

Hi Etroid,
1. Please fix the issues related to Pareview.sh and git link as has been pointed out by rrfegade and babusaheb.vikas.
2. In your .module file, as per me, there is no need of the the variable $added. You can omit it and directly add the js and css required.
Also the variables $css and $js can be safely ommited as they are required only for the purpose of invokation, and has not been used after that.

Multiple Applications

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

Project 1: https://www.drupal.org/node/2569879

Project 2: https://www.drupal.org/node/2493731

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.

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

I've gone through the checklist here and I believe @etroid has sufficiently hit all the marks. Notes inline:

Basic application checks: This application contains both a link to the project and the git command to clone the repo. The applicant's also demonstrated adequately (in my mind), that the module is not a duplicate. In particular, see notes in #6. As a module maintainer myself, I would probably be pretty annoyed if someone posted a patch that added a feature compatible with my module, but that re-architected half the module in order to do so.

Basic repo checks: The repository does indeed contain code.

Security Review: Because the applicant's module only exposes configuration options in the form of checkboxes and radios (and properly uses Drupal's form API and the Views API to do so), the module is not at risk of introducing XSS vulnerabilities. It also makes proper use of the permissions API, limiting access to the administrative interface to only those with access to Views' admin interface, so no access bypass vulnerabilities. Other vulnerability vectors seem largely irrelevant.

Licensing Checks: The module does not include a LICENSE.txt (as prescribed by Drupal standards), and does not contain non-GPL compatible code.

Documentation: The project page, README, and code contain adequate documentation. A site builder will know exactly what the module does, how to install it and configure it. A themer will know exactly how to override the behavior they need to, and the code was adequately commented, so that future contributors understand what's happening in the code.

Coding standards: Although the automated check by pareview.sh turns up multiple warnings and errors, I believe the module adequately follows coding standards. All of the PHP warnings have been addressed in #3 by the applicant (maintaining style parity with the Views module). The remaining errors are JavaScript related, but I don't believe this is something that needs to be addressed. Drupal itself doesn't have a JavaScript coding standard, and the code appears to follow an existing style, possibly this one. I believe the spirit of the coding standards checks are about ensuring high quality, secure, maintainable code, and I don't have reason to believe that this code is anything but that.

API and best practices Review: The module demonstrates a good understanding of core Drupal APIs and the Views API. One minor note I might add is the use of the static keyword in views_ajax_overlay_add_scripts(). Use of the drupal_static() pattern may be warranted, but I don't think this is a large enough issue to block acceptance.

Given all of the above, I'm moving this issue to RTBC.

Thanks for your contribution, Martin!

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.