Sandbox: http://drupal.org/sandbox/jasperknops/1535526
Git repository: git clone --branch 7.x-1.x http://git.drupal.org/sandbox/jasperknops/1535526.git hooker_hunter
Developed for Drupal 7.x

The hooker hunter enables you to search the usage of hooks throughout your project. If you search for a hook, a list of modules that implement this hook are displayed in the order they will be executed. If you search by module you will see a list of all hooks implemented by that module.

It is a tool for developers to quickly browse trough the life of hookers.

Comments

patrickd’s picture

Status: Active » Needs work

welcome,

Please read about the project application workflow.

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.
(At least this has to be done before switching back to needs review)

Please take a moment to make your project page follow tips for a great project page.

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/httpgitdrupalorgsandboxjasperknops1535526git

Moreover I'm afraid that project is too short to approve you as git vetted user. We are currently discussing how much code we need, but everything with less than 120 lines of (custom) code or less than 5 (functions that are not hooks just providing some information) functions cannot be seriously reviewed. However, we can promote this single project manually to a full project. Alternatively you can try to add more functionality to it so there's more code to review.

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

jasperknops’s picture

Status: Needs work » Needs review

Created new branch: git clone --branch 7.x-1.x jasperknops@git.drupal.org:sandbox/jasperknops/1535526.git
Updated code with violations described at http://ventral.org/pareview/httpgitdrupalorgsandboxjasperknops1535526git

Because it is just a small developers tool project I think the project page doesn't need more explanation?
I also have more ideas for features to add to this module. Do you want me to expand the module first before it can be approved?

Thanks!

patrickd’s picture

Even if this tool is for developers, please add short installation and usage instructions anyway, maybe also a list of features (even if there's currently only one) and a screenshot (you can upload it here: http://drupal.org/node/add/image).

We can make a full project out of it even if it is small, but it's probably not enough to give you vetted user permissions to promote projects yourself. Therefore, yes, some more features would be good ;-)

Please check the automated review again, it's still complaining about the missing readme and some little coding style issues you may could fix. (you can re-run it by using the repeat review tab).

jasperknops’s picture

I did everything you asked, I think ;)
Great tool btw, the automated review.

jasperknops’s picture

I added some extra functionality and ran the automated review again.

pgogy’s picture

The git URL should be - git clone --branch 7.x-1.x http://git.drupal.org/sandbox/jasperknops/1535526.git hooker_hunter (i think)

I get an error

Notice: Undefined variable: api_link in hooker_hunter_module_page() (line 28 of C:\xampp\htdocs\drupalclean\sites\all\modules\hooker_hunter\includes\hooker_hunter.admin.module.inc).

when module hunting

Also not sure on the difference between the tabs

"hook" and "hook menu" - perhaps could do with some text to explain?

Nice module though, like it as an idea.

jasperknops’s picture

I adjusted the git repo, removed the code that triggered the PHP Notice and added some extra form descriptions and a field_prefix.

ClaudeSchlesser’s picture

Status: Needs review » Needs work

Hello,

------------------------------------------------------------------------------

The description in the module list is not meaningful "Hunt the hookers by module or hook". There is no description at all in the configuration pannel. If I login to my administration area in a couple of months I might not remember what the module is for.

------------------------------------------------------------------------------

The README.TXT does not need a @file comment.

------------------------------------------------------------------------------

On the page "Enter the name of the hook you want to hunt for."

If the hook has no menu items (example: hook_permission), a recoverable fatal error is thrown:

Recoverable fatal error: Object of class stdClass could not be converted to string in hooker_hunter_hook_menu_page() 
(line 32 of /media/workspace/testsystems/drupal/1/modules/hooker_hunter/includes/hooker_hunter.admin.hook_menu.inc).

------------------------------------------------------------------------------

On the page "Enter the name of a module and you will see all menu items defined in MODULE_hook_menu()."

If the module that you enter has no menu hook, nothing appends. A notice should be displayed in this
case or the users will think that you module does not work as expected. Nothing happens? Hmmm?

------------------------------------------------------------------------------

This said, this is a very useful and well thought out module.
I will keep it for further use!

Regards,

jasperknops’s picture

Status: Needs work » Needs review

Hi,

I made the changes you asked for. I can't reproduce the error you had. Can you tell me again how I can trigger this. You say you are looking for menu items of module hook_permissions. Not very logical ;-)

Thanks!

ClaudeSchlesser’s picture

The issues that I discovered in my previous review seem to have been been fixed.

I can't reproduce the error you had. Can you tell me again how I can trigger this.
The error no longer occurs ;)

Sorry to be picky, but in my opinion you should rename the module to "Hook Hunter" (or something else).
Hooker is a slang word for prostitute ...

This said, I didn't see any other issues.

jasperknops’s picture

I do know what a hooker is ;-)
So you want me to change the name?

Is there something else I need to do to make it a real project?

ClaudeSchlesser’s picture

So you want me to change the name?

No, it's only a suggestion ;)

Is there something else I need to do to make it a real project?

In my opinion it would be ready for a real project with another name.

jasperknops’s picture

I renamed the module to hook_hunter.

crobinson’s picture

Status: Needs review » Reviewed & tested by the community

I know nit-picking over things like names seems tedious, but it's important to be culturally sensitive especially given how diverse Drupal's user base is.

I did a final code review on this project and found nothing blocking its release. I do have some minor comments but while I think they're worth addressing at some point, none of these fit the guidelines for "Needs Work"ing a project. Therefore, I'm marking this project "Reviewed and Tested."

1. In README.txt there are two spelling errors: "quickly browse trough", "to you module folder".

2. There is another one in includes/hook_hunter.admin.hook_menu.inc:49: "doesn 't" has a space before the apostrophe.

3. The same function's comment block doesn't provide a $returns. This isn't a hard requirement in any of the standards documents, and this function doesn't look like something others would really find useful to call since it's so form-related. But you provide an @param so I'm just making the suggestion for consistency to provide an @return. The same goes for the other two include files.

This looks like a really useful module - thanks for the contribution! I'm looking forward to its release.

jasperknops’s picture

I understand all nit-picking completely.

I adjusted your last remarks. What is the next step to make this an official project? I added a 7.x-1.0 tag to the repo.

Thanks a lot!

crobinson’s picture

Just wait a day or so. Being marked Reviewed & Tested was the next step. When they have time, one of the Drupal GIT Admins will make sure the reviewers didn't make any mistakes and then mark the ticket "Fixed". That will give you the access to convert your Sandbox project to a full project, and also to make new projects in the future without going through the same procedure. This is outlined here: https://drupal.org/node/532400

While you're waiting, you'd be doing the community a favor if you did the same kind of review on other projects. You don't need it any longer for the "bonus points" but there's a big issue queue and by now you must be familiar with the frustration that comes from such a long process! Here's a link to the queue:
http://drupal.org/project/issues/projectapplications?order=last_comment_...

patrickd’s picture

Assigned: Unassigned » patrickd
patrickd’s picture

Assigned: patrickd » Unassigned

When I hook-menu hunt for block module I get "Recoverable fatal error: Object of class stdClass could not be converted to string in hook_hunter_hook_menu_page()".

Also It can't find all existing hooks for some modules (eg. better batch implements multiple hooks like hook_menu(), hook_batch_alter but only hook_init() is found) - probably an issue with module_implements().

Some minor things I found (suggestions)

  1. It's usual that the root menu item (HOOK_HUNTER_MENU_ITEM) is a 'system_admin_menu_block_page', in your case it's the same like the hook-list page - but do it how you like it best.
  2. 'ajax/' . HOOK_HUNTER_MENU_ITEM I wouldn't prefix the path with ajax/ here as you could possibly collide with other menus here - it's usual to just suffix it with /autocomplete
  3. hook menu hunting shows an empty table telling me "This module doesn't implement hook_menu()." but I didn't enter anything yet

Leaving rtbc as I'm too tired to continue at the moment, sorry.

patrickd’s picture

Status: Reviewed & tested by the community » Fixed
  • "MODULE_hook_menu()" hmmm - I don't think this one exists ;-)
  • $items = call_user_func($function); you should check first whether this function exists.
  • To show that there's no item found you print out a row with maximum colspan - but note that there is the "empty" variable in theme_table() you could use for this
  • To check whether it's a core module you can also have a look into your $module_data variable: $module_data[$module]->info[package] == 'Core';
  • Wrong error message for searching for hooks implemented by a module: t('No modules were found that implement this hook.')
  • Even when it's unlikely that module names contain harmful stuff, please make sure that the module's human readable name ($module_data[$module]->info['name']) and the menu-title/description is sanitized by check_plain()

Generally doing some more inline-documentation wouldn't hurt you ;-)

After all (please add the mentioned check_plain()s before creating your fist release) this looks good enough for me.

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 depending on which you feel is best - don't forget this power brings responsibility.

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.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

updated git repo