Menu position vocabulary

Hi,

you havent committed your .module and other files. I see only .info file here.

Thanks
Parwan

Hi,

Here are the details from your manual review done :-

1) You do not have @doc file comment in your .module file.
2) You have commented code in your .inc file. It should be removed.
3) Also proper indenting and formatting of code should be done. As mentioned here.

Also make sure you end each of your file with a new line. And also please remove LICENSE.txt file. It will be added automatically.

I think you shouldn't be having the master branch. That should be removed. That is also mentioned in automated review here.

Thanks

This application is not fixed? See http://drupal.org/node/532400

Hi Spaiz,

First of all, thanks very much for committing this module. This functionality seems to be good complement to the existing menu_position module.

I have reviewed your module quickly and it seems everything is fine, except one comment I would have:

In file menu_position_vocabulary.inc, at lines 21 and 57, it seems you're calling functions from the taxonomy module.
In particular:
taxonomy_term_load
taxonomy_vocabulary_get_names

So I would simply recommend that you add one line of code in your file menu_position_vocabulary.info:
Line 2, for example, add the following code:

dependencies[] = taxonomy

Otherwise, I'm afraid there would be a risk of getting a Fatal error: Call to undefined function Error if the Taxonomy module is not enabled.

So far, that's all I could find, but I will keep looking and certainly let you know if I can find anything else.
I hope this comment will help you to improve this nice module.

Cheers!

I don't see any issues with the code. A couple comments about the project in general:

1. The project page is a bit sparse. Take a look at Tips for a great project page
2. From http://drupal.org/node/447604 (emphasis added):
   

   Every contributed module should provide a README.txt in the package. This file should contain a basic overview of what the module does and how someone may use it.

   The module seems pretty easy to use but perhaps you could add a "usage" section to the README.txt to fulfill this requirement.

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 :-)

README.txt
ok, better add a to-do list so that others can make contribution

project page
ok

Master Branch
You are not using it

Major coding standards / best practice issues
Manual Review: Everything is fine

License
Better to have it

access administration vs. administer site configuration
Permissions make sense.

PAReview: 3rd party code
No 3rd party stuff here.

Multiple Applications
No

Already Approved
Nope, but I think you will be soon.

Idle Application
Nope, you're all set.

Code too short
Nope, you're all set.

Wouldn't it be better to provide this functionality as a patch for Menu position?
Additionally the module is quite short. We are currently discussing how much code we need, but everything with less than 120 lines of code or less than 5 functions cannot be seriously reviewed. However, we can promote this single project manually to a full project for you.

I want to maintain this functionality as a standalone module.

Can you explain why?
We prefer collaboration over competition. If the functionality provided by a module is not too fundamental for patching the one it extends, we would love to see you joining forces and concentrate all power on enhancing one module.
I'm sure the maintainer of Menu position will be glad to see this as a patch or welcome you as a co-maintainer if he does not have enough time to maintain the module anymore.

120 lines of code? If you want, I can make 500 lines of code for it ;) Don't judge about module by number of lines...

Please see the discussion about how much code we need for a review.

Hi,

This module has been reviewed and tested. The necessary changes have been made and the module indeed does exceed 120 lines of code in total. Nice work and I look forward to using this as a full project!

Some notes:

* You could pass a path to arg() to create your $arg variable, rather than exploding it yourself, to make your module code a bit simpler.
* This module implements hooks from the menu_position module. Those hooks were added to allow menu_position to be extended by other modules. This is not competition; it's Drupal's modular architecture in practice.

The arg() thing isn't a blocker, so I'm moving this to fixed.

Spaiz, thanks for your contribution and welcome to the community of project contributors on drupal.org.

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects, at your discretion.

Now that you've experienced the full review process, please consider reviewing other projects that are still awaiting review. Anyone can help with reviews, following the guidelines.