[D7] Disqus Custom Identifier

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

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 vilepickle,

I have corrected the GIT url in your application. It should be http://git.drupal.org/sandbox/vilepickle/2114383.git

Cheers,
Ash

Hi mate,

As per drupal guidelines your module should have a minimum of 120 lines of code or 5 functions before it could be reviewed.
Please refer to section 2.3 of this page https://drupal.org/node/1587704

Good luck

Cheers,
Ash

Wrong, of course we can review smaller modules, too. We just can't give the git vetted user role away on a short projects, but we can always promote it manually to a full projects and applicants can always come back to us when they have a new project for promotion.

I think I will just remove that paragraph from the review checklist, since that is by no means an application blocker.

Thanks for clearing that up klausi...

I have reviewed the module, and can't find anything wrong with it.
I think this project can be promoted to a full project.

On second thought:

Your module does not implement hook_uninstall().
You should implement hook_uninstall() to delete any variables your module creates like disqus_custom_identifier etc.

Looks good.

Automated Review

Best practice issues identified by pareview.sh / drupalcs / coder. There are findings at http://pareview.sh/pareview/httpgitdrupalorgsandboxvilepickle2114383git

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

(*) Maybe: Does not cause module duplication and fragmentation. Have you approached the maintainer(s) of https://www.drupal.org/project/disqus and suggested a patch to the main disqus module?

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements

3rd party code

Yes: Follows the guidelines for 3rd party code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and the README Template. However, the documentation is a little dated for the D7 branch as it still mentions CCK fields. Please updated the D7 branch's README.

Coding style & Drupal API usage

1. (+) Why doesn't this module hook_form_alter the previous disqus admin settings form (disqus_admin_settings)? No need to create a new menu callbacks.
2. (*) I receive a wall of warnings/errors when I visit the admin settings page.
   

   Notice: Undefined index: remove_normal_disqus in disqus_custom_identifier_admin_settings() (line 37 of sites/all/modules/disqus_custom_identifier/disqus_custom_identifier.admin.inc).
   Notice: Undefined index: blog in disqus_custom_identifier_admin_settings() (line 47 of sites/all/modules/disqus_custom_identifier/disqus_custom_identifier.admin.inc).
   Notice: Undefined index: blog in disqus_custom_identifier_admin_settings() (line 53 of sites/all/modules/disqus_custom_identifier/disqus_custom_identifier.admin.inc).
   Notice: Undefined index: poll in disqus_custom_identifier_admin_settings() (line 47 of sites/all/modules/disqus_custom_identifier/disqus_custom_identifier.admin.inc).
   Notice: Undefined index: poll in disqus_custom_identifier_admin_settings() (line 53 of sites/all/modules/disqus_custom_identifier/disqus_custom_identifier.admin.inc).
   Notice: Undefined index: forum in disqus_custom_identifier_admin_settings() (line 47 of sites/all/modules/disqus_custom_identifier/disqus_custom_identifier.admin.inc).
   Notice: Undefined index: forum in disqus_custom_identifier_admin_settings() (line 53 of sites/all/modules/disqus_custom_identifier/disqus_custom_identifier.admin.inc).
   Notice: Undefined index: panel in disqus_custom_identifier_admin_settings() (line 47 of sites/all/modules/disqus_custom_identifier/disqus_custom_identifier.admin.inc).
   Notice: Undefined index: panel in disqus_custom_identifier_admin_settings() (line 53 of sites/all/modules/disqus_custom_identifier/disqus_custom_identifier.admin.inc).
   Notice: Undefined index: article in disqus_custom_identifier_admin_settings() (line 47 of sites/all/modules/disqus_custom_identifier/disqus_custom_identifier.admin.inc).
   Notice: Undefined index: article in disqus_custom_identifier_admin_settings() (line 53 of sites/all/modules/disqus_custom_identifier/disqus_custom_identifier.admin.inc).
   Notice: Undefined index: book in disqus_custom_identifier_admin_settings() (line 47 of sites/all/modules/disqus_custom_identifier/disqus_custom_identifier.admin.inc).
   Notice: Undefined index: book in disqus_custom_identifier_admin_settings() (line 53 of sites/all/modules/disqus_custom_identifier/disqus_custom_identifier.admin.inc).
   Notice: Undefined index: page in disqus_custom_identifier_admin_settings() (line 47 of sites/all/modules/disqus_custom_identifier/disqus_custom_identifier.admin.inc).
   Notice: Undefined index: page in disqus_custom_identifier_admin_settings() (line 53 of sites/all/modules/disqus_custom_identifier/disqus_custom_identifier.admin.inc).
   Notice: Undefined index: webform in disqus_custom_identifier_admin_settings() (line 47 of sites/all/modules/disqus_custom_identifier/disqus_custom_identifier.admin.inc).
   Notice: Undefined index: webform in disqus_custom_identifier_admin_settings() (line 53 of sites/all/modules/disqus_custom_identifier/disqus_custom_identifier.admin.inc).
   

3. (*) disqus_custom_identifier.info: Duplicate Key in Section (at line 6)

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.

Git clone command for the sandbox is missing in the issue summary, please add it.

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

Duplication

This sounds like a feature that should live in the existing disqus project. Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition. Please open an issue in the disqus issue queue to discuss what you need. You should also get in contact with the maintainer(s) to offer your help to move the project forward. If you cannot reach the maintainer(s) please follow the abandoned project process.

If that fails for whatever reason please get back to us and set this back to "needs review".

If the maintainers of disqus module feel similarly, then there's no reason this can't be a separate module. But let's let them help us make that decision. However, if you wish to withdraw this request, that is your option.

Cross-posting. Based on #1175668-38: Add an option to override disqus_identifier, this functionality should be placed in the parent module.