Cookie Crumbs

Couldn't find much. Nicely done.

Only thing you should change is remove the cookie_crums.theme.inc file and move the content to the module file. One file less that needs te be loaded. The module file won't be less readable with the contents of the theme file.

And on the side: Thanx for your comment on Swift Pics where you link to the doc about the Entity API. Didn't knew that yet! Saved it!

README.txt suggests that we can check status report for more details of library exists, but module can't be enabled as hook_requirements don't check phase == 'runtime' and is invoked during install and shows library missing, this can be fixed by adding check for $phase.

You may Implement hook_help to add help of the module.

Their are few points in README.txt which don't have a explanation (IRC API Hooks, Cache Warning, Design Decisions) which should be completed or removed if not applicable.

Regards,
Sadashiv.

Looks good, trogels.

As @sadashiv writes the README.txt contains a table of contents which is out of sync with the actual content (looks like copy'n'paste from some other module).

I don't know how the hansel-script works but maybe you could consider adding some options to the drupal_add_js() calls?

I.e. would it be possible to add a 'scope' => 'footer'? That would be better for client side performance.

Also consider whether setting 'group' => JS_LIBRARY for the library.

README.txt tells me to go to admin/reports/status to find out where to download the library. Why not just put this line:

Place jquery.hanselsrevenge.js in sites/all/libraries/hanselsrevenge. You can download the library from https://github.com/soitgoes/hanselsrevenge

directly into README.txt?

Also in README.txt, I think 'herical' should be spelled as "hierarchical."

README.txt and the project description would benefit from a more detailed description of exactly what this module does. I would recommend a sample use case or example or something. Maybe it's kind of obvious, but I had to look through the source code to even figure out that it adds a block and that I could configure the number of links to be shown.

Otherwise, thanks for sharing, this is pretty cool!

Hi,

breadcrumps can be quite cumbersome sometimes so this approach looks very promising to get the real-life navigation breadcrump.

README: At first look I was confused why you referenced https://github.com/soitgoes/hanselsrevenge as there is already a JS file shipped with the module that has the name "hanselsrevenge" in it. Would it make sense to rename the shipped JS file to sth like cookie_crumbs.js?

Other than that it looks great. Well done!

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

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

  FILE: /home/klausi/pareview_temp/cookie_crumbs.module
  --------------------------------------------------------------------------------
  FOUND 2 ERROR(S) AFFECTING 1 LINE(S)
  --------------------------------------------------------------------------------
   41 | ERROR | Incorrect spacing between argument "$delta" and equals sign;
      |       | expected 1 but found 0
   41 | ERROR | Incorrect spacing between default value and equals sign for
      |       | argument "$delta"; expected 1 but found 0
  --------------------------------------------------------------------------------
  

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. "variable_set('cookie_crumbs_length', check_plain($edit['cookie_crumbs_length']['value']));": "When handling data, the golden rule is to store exactly what the user typed. When a user edits a post they created earlier, the form should contain the same things as it did when they first submitted it. This means that conversions are performed when content is output, not when saved to the database" from http://drupal.org/node/28984
2. "module_load_include('module', 'cookie_crumbs', 'cookie_crumbs');": no need to use module_load_include as you are including files from your own module which you already know where they are. Use something like require_once 'mymodule.inc';
3. The JS library is vulnerable to XSS exploits. If I have a node with the title <script>alert('XSS');</script> I get a nasty javascript popup as soon as I navigate to that node. The library needs to sanitize user provided text before printing.

I don't know if you are the author of that library or if you can fix it, but security issues are application blockers. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Please don't remove the security tag, we keep that for statistics and to show examples of security problems, even if it seems solved now.

Looks RTBC to me now! Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Trying to remove review bonus tag again ...

no objections for more than a week, so ...

Thanks for your contribution, trogels!

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.