entity uuid

You need to set the status to "needs review" if you want to get a review.

You have not listed any reviews of other project applications in your issue summary as strongly recommended here: http://drupal.org/node/1011698

Automated review:
- Fix the issues from PAReview.sh: http://ventral.org/pareview/httpgitdrupalorgsandboxzkday1531710git

Manual review:
- Write a README.txt file with more info about how to use your module.
- Otherwise this module has just a few lines of code and it looks good for me.

Hi,

There are still some errors currently detected by pareview : http://ventral.org/pareview/httpgitdrupalorgsandboxzkday1531710git

You use hook_modules_installed() to call hook_install(), why? hook_install is automatically used during installation.

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.

Hi,

Files should be removed from the master branch.

It is normal (but not compulsory) to put admin code into an admin.inc file (form, form submit etc.)

Including a configuration path in the .info file provides a quick link from the modules page after installation: configure = admin/config/system/entity-uuid

The configuration page is very bare, just a list of checkboxes and labels, you could put some information as to what exactly will happen when you click Save. (To be fair, the UUID module configuration page is equally as bare)

Does your module require it's own package? "package = entity". Entity API is in 'Other', UUID is in 'UUID', I would suggest leaving package out altogether to put it in 'Other', but even putting it together with 'UUID' would be better than a new package called 'entity'.

I find the install functions confusing. Do you need to call _entity_uuid_install_uuid_fields() on installation (hook_install) and after installation (hook_installed) as well? The same function is called by your configuration form_submit method also.

Might it not be better to create a common function that can be called by both hook_install and form_submit? You could move the function _entity_uuid_install_uuid_fields() out of the .install file and rename it entity_uuid_create_uuid_fields() (no underscore) in the .module file. This function is not solely a module installation function as it acts in other capacities, it 'creates' fields, it doesn't really 'install' anything. Create a new function entity_uuid_settings_form_submit() and call entity_uuid_create_uuid_fields() from that function. Remove module_load_include('install', 'entity_uuid') from the new form_submit function.

entity_uuid_settings_form($form, &$form_state) has a confusing comment: "Menu callback: options for UUID." It is really "Configuration settings form".

The README.txt file could provide a lot more information, including a link to your project page, and be formatted better, e.g.

CONTENTS OF THIS FILE
---------------------

* Introduction
* Installation
* etc.

Otherwise its fine. Some very basic testing returned no errors. Good luck! :)

Forgot to change status

Ok, most of my points have been addressed, well done and thank you!

The only very minor thing I noticed was that you could wrap the menu/title and description with t() as you have done with the form functions.

I haven't tested the functionality in-depth as it is not something I use, but have not encountered any errors or problems with installation, configuration or uninstallation although it doesn't actually do anything to my entities... but looking at the code I think that perhaps I need to understand how the UUID module works first. I use a lot of entities and it might be something I will come back to in the future. It really might be worth getting some decent documentation together for this.

Afaic this is probably RTBC but I'm not sure if I'm allowed to change it to that status or if someone needs to check my review?

Just found a bug on uninstall. (After disable, do 'uninstall' and 'confirm')

Fatal error: Call to undefined function uuid_get_core_entity_info() in E:\xampp\htdocs\adlibtools\sites\all\modules\entity_uuid\entity_uuid.module on line 32

Looks like you will need to load the uuid module for your uninstall routine.

Sorry to pop your balloon!

You're welcome!

Tested and bug fixed.

Btw, module_load_include('entity.inc', 'uuid') imo should be module_load_include('inc', 'uuid', ''entity') (it works either way but first param is $type which I read to mean extension) and you might want to test that the includes were loaded to reduce risk of fatal error:

  // Make sure all function that are called in this function is exists.
  if (module_load_include('module', 'entity_uuid')
  && module_load_include('inc', 'uuid', 'uuid.entity')) {

You are probably right but I always imagine the user deleting or moving files and folders randomly... and then imagine how my module would handle it! ;)

manual review:

• "'#markup' => $value['label'],": this could be user provided input if a module like ECK is used, therefore you need to sanitize the label before printing it. See http://drupal.org/node/28984
• You just prepare the database for storing the UUIDs, but shouldn't you also implement hook_entity_presave() and generate a UUID if it is missing?

Anyway, I consider the first point only a very minor security issue that should not hold back this application, so ...

Thanks for your contribution, zkday! 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 depending on which you feel is best.

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.

As you continue to work on your module, keep in mind: Commit messages - providing history and credit and Release naming conventions.

Thanks to the dedicated reviewer(s) as well.