Rename Modules

http://ventral.org/pareview/httpgitdrupalorgsandboxgiovanniglass1485844git

Your not working correctly with branches to create project releases. 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.

A few things to improve:
http://ventral.org/pareview/httpgitdrupalorgsandboxgiovanniglass1485844g...

It should be familiar with: Coding standards

fix project link

Please don't change to needs work if there are only coding style issues,
most of these are suggestions and should not block deeper manual reviews.

Any reason why you're not updating the menu link name yourself?
Also that 1 query you're doing should not use * if you're only going to use 1 field

Besides that seems to work.

Hello ,

Manual Review :

rename_modules.module :
1.If possible, don't put any HTML into t() functions, this will always make the live easier for translators.
2.do not create link markup yourself, use l() instead in function rename_modules_library() , function rename_modules_libraries_info() and also in other functions.
3.function rename_modules_help() contains $output = '';what is the need for this?you can remove this, it wont results in error it seems else please add a comment.

rename_modules_admin.inc:
Please add module/file name as prefix to the function name to avoid name clashes.

Thanks

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

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

With "application" i was referring to this issue, which is about you getting the git vetted user role. See also http://drupal.org/node/1011698

It seems there is a dependency that isn't mentioned in the README:

The jquery_replacetext library could not be found. Please check if it is installed correctly.

It might not be a bad idea to let the user know where to get the library if they do not have it after enabling the module.

After installing the needed libraries, I enabled and used the module without a hitch. It works as designed, although the spans in the message output is a little annoying and are getting filtered.

I was referring to the drupal_set_message that popped up. I marked it as tested as well.

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

Thanks for the contribution, momendo. Sorry that this seems to have gotten lost in the pile for so long.

I've taken a look through the code, and had a few questions and suggestions.

Libraries dependency

In the Installation instructions, you specifically instruct folks to install Libraries 7.x-2.x-dev. Because the -dev version can and will change over time, I wouldn't recommend telling folks to always use it, unless there is a specific reason why they shouldn't use the currently recommended 7.x-2.x release at any given point in time. If there is a reason, I'd still put the 7.x-2.x text in the instructions, and then add a temporary note (with a date) to the instructions, stating to use the -dev version.

While your info file specifies libraries (>=2), the examples in the writing .info files documentation seem to indicate that the best practice would be to specify the .minor version as well. So I would update this to read libraries (>=2.0).

span class="no-touch" in your .info file

This one bothers me ... I understand why it is there, but it seems there has to be a better way. I'd suggest entering the text as 'M_odules', or 'M0dules', or 'Modu1es' in the info file instead. Having html tags inside the info file is well outside of the anticipated .info file use cases ... not only is there a risk that this will affect drupal.org's automated packaging and parsing of .info files, but I'm pretty sure this will wreck havoc with project_dependency and our automated testing infrastructure as well.

rename_modules.drush.inc

I'd recommend updating the comments in your docblocks, making them more specific to your module instead of the default drush example boilerplate.

Otherwise, things look okay ... if you're able to respond to the points above (the span issue in particular), I'd be ready to promote this.

Changes look good. One minor nitpick:

* drush command callback. Download the jQuery Replacetext library.

I'd reword this as "Downloads the jQuery Replacetext library. (Drush command callback)", so that it looks more complete and can stand on it's own as a comment. Sites such as api.drupalcontrib.org aggregate and expose these docblock comments without the associated context provided by the code ... and given the current wording/capitalization, it almost appears as if it's missing half a sentence from the line above.

Thanks for your contribution, momendo!

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.

Uh, a little late to this but how is the functionality of this module not just a tiny subset of String Overrides http://drupal.org/project/stringoverrides?

This premise behind this module looks like code duplication to me.

I had the same thought ... but I think there is room for the small and focused solution in addition to the larger flexible approach. We do want to prevent the creation of duplicate modules, but I don't believe in blocking duplicate 'functionality' if the two modules in question have different scopes (and arguably, different audiences).

mmk