[D7] Menu per language

welcome,

• 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.
• Remove "version" from the info file, it will be added by drupal.org packaging automatically.
• Remove "project" from the info file, it will be added by drupal.org packaging automatically.

As installation and usage instructions are quite important for us to review, please take a moment to make your project page follow the tips for a great project page. Also create a README.txt that follows the guidelines for in-project documentation.

while waiting for an in-depht review of your module you can start out fixing some coding style issues detected by automated tools:
http://ventral.org/pareview/httpgitdrupalorgsandboxmschuddings1561010git

Also all your code is indented with 2 spaces - but it's in global space, so there should not be any indentation.
I mean:

<?php

  /**
   *
   */
  function ...

should be

<?php

/**
 *
 */
function ...

We do really need more hands in the application queue and highly recommend to get a review bonus so we can come back to your application sooner.

regards

Hello.

• Your master branch is clean. Good!
• Ventral is not working now, but I found some minor issues with the comments. Please check http://drupal.org/node/1354 to help
• Fast tip: Add full stops in your comments.
• I'm reviewing 7.x-2.x branch

Hello again, my personal review!

• function menu_language_init(): If you need you CSS on every single page request you should specify that in the info file. See http://drupal.org/node/756722
• You're using two types of permissions 68:'access arguments' => array('access administration pages'), and 77:'access arguments' => array('administer site configuration'),, why? If this is wrong, use your own permission.
• On info file dependencies[] = variable is not necessary, because it's called by i18n.
• Don't forget to add the review bonus tag and to add some other review links to the issue summary.

Please update your last reviews and add the review bonus tag.

Manual review (reviewed 7.x-2.x):

Branches?

You have to branches - 7.x-1.x and 7.x-2.x, which one do you want us to review?

Similar?

Are there similar projects? Which are they, and how is your different? For instance
i18nmenu in the i18n module?

Variable names

Variables should be prefixed with your modules name

Remove variables

when you add variables, it is good practice to remove them if the module is deactivated, this you could do in a hook_uninstall in the a .install file

Empty $title?

In _menu_language_menulanguage_form you are declaring an empty value to $title, why?

Better comments needed

In menu_language_get_menu_links_by_language much things is going on that I really do not understand, I think better comments would solve that, just JOINS, and FIELDS does not say anything for real.

Requirements?

In you README it says that Internationalization module, Translated menu's (which module is that?) and Variable module is needed, in you info, only i18n is declared as dependency

Review of the 7.x-2.x branch:

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

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.

Please check your code with the Coder Review module. It find some problems on the most sensitive setting (don't worry about the $menu_admin message; that's a bug in Coder).

Licensing: No problems noted.

Some of your comments are written in ALL CAPS. Please avoid this. Use complete sentences with punctuation.

In this comment, "Shows all the ena menus in the ena languages." does "ena" mean "enabled"? Please use full words.

You can add blank lines inside functions to divide up sections that go together. Functions should always be followed by a blank line.

It is not clear to me from the documentation what the module does.

@Liam Morland, it is great that you do reviews, but please review some which is not in needs work state already. We have a lot of them ;-)

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.

This looks like a feature that is already provided by the i18n module or possibly others: http://drupal.org/project/i18n . Module duplication and fragmentation is a huge problem on drupal.org. Please open an issue in the i18n queue to discuss possible improvements before creating your own module. Get in contact with the maintainer(s) to address this, if necessary.

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

Hello Mschudders,

Your sandbox project still contains a master branch. Refer to this link http://drupal.org/node/1127732 and remove master branch.

Ok, so I guess this could go as separate module. I would suggest to rename it to i18n_menu_overview or similar, so that it is clear that this is just a small addition to i18n_menu.

There is still a master branch, make sure to set the correct default branch: http://drupal.org/node/1659588 . Then remove the master branch, see also step 6 and 7 in http://drupal.org/node/1127732
Review of the 7.x-2.x branch:

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

  
  FILE: /home/klausi/pareview_temp/theme/menu_language_page.tpl.php
  --------------------------------------------------------------------------------
  FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
  --------------------------------------------------------------------------------
   5 | ERROR | File doc comments must be followed by a blank line.
  --------------------------------------------------------------------------------
  

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. menu_language_menu(): do not document parameters again on hook implementations, see http://drupal.org/node/1354#hookimpl
2. configuration page: "'access arguments' => array('access administration pages'),": The administration menu callback should probably use "administer site configuration" - which implies the user can change something - rather than "access administration pages" which is about viewing but not changing configurations.
3. "'access callback' => TRUE,": that means that any anonymous user can access the page. Information leakage, which means this is a security issue. Remove that access callback entry and the default user_access() callback will be used and the permission will be taken into account.
4. _menu_language_goto_menu(): doc block format is wrong, see http://drupal.org/node/1354#functions . Same for _menu_language_menulanguage_form().
5. menu_language_form.inc: looks like a lot of copied code. Please add @see references to the orignal core function where you stole the code.

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

Multiple Applications

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

Project 1: http://drupal.org/node/1561062
Project 2: http://drupal.org/node/1793098

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.