Nodewords Google

Welcome,

It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.

Review of the master branch:

• README.txt is missing, see the guidelines for in-project documentation.
• Drupal Code Sniffer has found some issues with your code (please check the Drupal coding standards). You can check here: http://ventral.org/pareview/httpgitdrupalorgsandboxbalrajb1819934git

You may want to add a cleanup operation in the install file - as per nodewords_basic, to get rid of the tags from the nodewords table.

I reviewed the code manually and I couldn't find anything that seems wrong to me (but I'm quite new to programming modules). Regarding coding standards, it looks good to me. The idea of the module sounds very important to me.

What I am thinking, though:

1. Why does this have to be a new module? Can't your code be integrated into the nodewords module?
2. There is also the Google News Sitemap module, but as far as I can see, it works differently since it provides a site map, not a meta tag.

I'm not sure, but I think there is no reason to set the priority to 'major', or is there? To get more attention, you could participate in the Review bonus program.

I have manually reviewed the code of the module, and tested the module with a clean installation of Drupal 6. The module functions as described.

One comment ... nodewords_google_uninstall() seems more complex than needed ... since there's only one value in the array, I'd suggest just hardcoding 'news_keywords' into the query.

Thanks for your contribution, BalrajB!

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.

BalrajB:

No need to close the issue ... once an item is moved to 'fixed', the site will automatically close it after two weeks of inactivity.

The difference between the two is that 'fixed' will still appear in people's trackers and many default views on the website, whereas 'closed (fixed)' will not. Thus, if we move items directly to 'closed (fixed)', and it doesn't appear in people's trackers, they could miss the fact that it has been resolved ... by using 'fixed' and auto-closing, it remains on trackers for long enough that people can see the status update.

While it is not as important here, this is the process that is used across the rest of the Drupal.org website, so I wanted to ensure you were aware of it. :)