Suggest Similar Titles

Don't forget to add the "PAReview: review bonus" tag as indicated in #1410826: [META] Review bonus, otherwise you won't show up on my high priority list.

There is still master branch, please remove it.
Also your module defines variables by using system_setting_form(), you should remove them in hook_uninstall.

I'm not sure if using hyphens in form item names match drupal coding standards:
$form['fieldset-suggest-title']

they usually with underscores:
$form['fieldset_suggest_title']

But as I said I'm not sure so forgive me if I'm wrong.

i think it's better to remove admin settings form to .inc file.

Changed priority, see http://drupal.org/node/539608 - Application Review Timelines

Still some issues(errors+warnings) with your code (please check the Drupal coding standards)

please use Coder module before pushing your code to git.

I have patched your module at #1931750: Move admin config form page to admin.inc file.
If you considered using this patch, I would greatly appreciate if you could grant me the authorship on this commit, since I would like to potentially keep track of the patches I submitted that were successfully able to be transformed into commits. See user profiles, for example mine would be:

Pass the following option to git commit to attribute authorship to this user:
--author="ycshen <ycshen@2259178.no-reply.drupal.org>"
Learn more about proper Git attribution.

You need to set the status to "needs review" if you want to get a review, see http://drupal.org/node/532400

Besides, comment from #9, by ycshen, referring to #1931750: Move admin config form page to admin.inc file, seems to be relevant and the indicated patch looks good as well.
Therefore, I would recommend that you take a look at this issue first and try to fix the problem before changing the status back to needs review.

Feel free to let us know if you would have any questions, comments or objections on this issue, we would be glad to explain further.
Hopefully with these successive rounds of reviews we should be able to greatly improve the overall quality of this project.

Thanks again for contributing this nice module.
Cheers!

Please try to fix issues reported by automated review tools first: http://ventral.org/pareview/httpgitdrupalorgsandboxtanzeel1929360git

Ran your code through PAReview and all looks good, on manual review everything looks good. One suggestion I would make is to move the logic that is generating the node links into a preprocess function rather than having this in the .tpl.php function

 l($n->title, 'node/' . $n->nid, array('attributes' => array('target' => '_blank'))) ?>

This could go into a preprocess function and then the line would simply be something like:

<li><?php echo $node->link; ?></li>

Other than that everything looks good.

manual review:

1. suggest_duplicate_title.js: @file doc block is useless, please improve it. What is this JS used for?
2. suggest_duplicate_title.js: Do not use $(document).ready(function(), use Drupal.behaviors instead. See http://drupal.org/node/304258
3. suggest_duplicate_title_uninstall(): variable_gel() does not exist?
4. suggest_duplicate_title_ajax_title(): why do you call exit; here? Please add a comment.
5. suggest_duplicate_title_ajax_title(): is REGEXP official SQL standard? Will that work on Postgresql? Why can't you use LIKE? Please add a comment.
6. suggest_duplicate_title_ajax_title(): the check_plain() calls are useless here since you are not printing the result to the user. Please read http://drupal.org/node/28984 again.
7. suggest_duplicate_title_admin_settings(): variables typically use under scores instead of dashes.
8. suggest_duplicate_title_admin_settings(: this is vulnerable to XSS exploits. If I have a node type with the title <script>alert('XSS');</script> I will get a nasty javascript popup on the admin config page. You need to sanitise all user provided text before printing, see also the writing secure code handbook page.

Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Please add all your reviews to the issue summary so that we can track them.

Review of the 6.x-1.x branch:

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

  
  FILE: /home/klausi/pareview_temp/suggest_duplicate_title.admin.inc
  --------------------------------------------------------------------------------
  FOUND 0 ERROR(S) AND 2 WARNING(S) AFFECTING 2 LINE(S)
  --------------------------------------------------------------------------------
   55 | WARNING | Do not pass empty strings to t()
  --------------------------------------------------------------------------------
  
  
  FILE: /home/klausi/pareview_temp/suggest_duplicate_title.module
  --------------------------------------------------------------------------------
  FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S)
  --------------------------------------------------------------------------------
   38 | WARNING | Line exceeds 80 characters; contains 88 characters
  --------------------------------------------------------------------------------
  
  Time: 0 seconds, Memory: 8.00Mb
  

• DrupalPractice has found some issues with your code, but could be false positives.
  

  
  FILE: /home/klausi/pareview_temp/suggest_duplicate_title.admin.inc
  --------------------------------------------------------------------------------
  FOUND 0 ERROR(S) AND 5 WARNING(S) AFFECTING 5 LINE(S)
  --------------------------------------------------------------------------------
   34 | WARNING | All variables defined by your module must be prefixed with your
      |         | module's name to avoid name collisions with others. Expected
      |         | start with "suggest_duplicate_title" but found
      |         | "suggest_title_settings"
   40 | WARNING | All variables defined by your module must be prefixed with your
      |         | module's name to avoid name collisions with others. Expected
      |         | start with "suggest_duplicate_title" but found
      |         | "suggest_title_noof_nodes"
   47 | WARNING | All variables defined by your module must be prefixed with your
      |         | module's name to avoid name collisions with others. Expected
      |         | start with "suggest_duplicate_title" but found
      |         | "suggest_title_node_access"
   61 | WARNING | All variables defined by your module must be prefixed with your
      |         | module's name to avoid name collisions with others. Expected
      |         | start with "suggest_duplicate_title" but found
      |         | "suggest_title_ignored"
   67 | WARNING | All variables defined by your module must be prefixed with your
      |         | module's name to avoid name collisions with others. Expected
      |         | start with "suggest_duplicate_title" but found
      |         | "suggest_title_percentage"
  --------------------------------------------------------------------------------
  
  Time: 0 seconds, Memory: 3.25Mb
  

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. suggest_duplicate_title_admin_settings(): variable naming: also the node type specific ones should start with your module's name.
2. If I enter a title and click the save button immediately after I will never see the duplication box. Not sure how that could be fixed, but I was expecting node title duplicate suggestions as soon as I started typing in the field.
3. title-suggestion.tpl.php: all user facing text must run through t() for translation.

Although you should definitively fix those issues they are not super important application blockers, so I guess this is RTBC. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

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

Typo fixed

no objections for more than a week, so ...

Thanks for your contribution, tanzeel!

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.