[D7] Menu Auto Display

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxvgriffin2388337git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

Automated Review

No issues found

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

Yes: Does not cause module duplication and/or fragmentation.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements.

3rd party assets/code

Yes: Follows the guidelines for 3rd party assets/code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and/or the README Template.

I would add that I found the README a bit unclear as to what this module does.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements. Used db_queries.

Coding style & Drupal API usage

1. Make the module description more clear.
2. Allow for appending page content below the list of children links

This review uses the Project Application Review Template.

@ashopin From your post, it is not clear whether or what parts of project need review. Where are the items with + and * signs ? I think you should change the status to either 'Needs work' or 'Reviewed by the community'.

@smido: My apologies, I forgot to change the status.

1. FILE: ...review/sites/all/modules/menu_auto_display/menu_auto_display.inc
   ----------------------------------------------------------------------
   FOUND 1 ERROR AFFECTING 1 LINE
   ----------------------------------------------------------------------
   98 | ERROR | [x] Doc comment long description must end with a full stop.
2. Similar to #4.1, I'm not really sure how to use this module. I've enabled it and can't get the content to render to check some security things I like to do. I'm marking as needs work for that reason.

manual review:
project page could be a bit more specific with a use case example and screenshot, see also https://www.drupal.org/node/997024 . I also found it confusing how to get this to work, so could add a recipe with steps like "create node 1" then "create node 2" then create "menu item X" then "add menu link for node 1" etc.

Since most of the code is copied from Drupal core this project on its own is too short to approve you as git vetted user. I'm looking at you other sandboxes now.

gauth_svc_acct:
"variable_get('gauth_svc_acct_email_address', '')": all variables defined by your module must be removed in hook_uninstall().

Have to run now, will continue later.

Hi vgriffin,
your README.txt may be confusing for many users, especially the newbies.. so please modify your README, and include how to use the module preferably step wise. You may also include snapshots.
You may refer to modules like views_slideshow, admin, etc . which is found quite useful for the users.

Also please modify your project page as is mentioned by klausi.

Coding Style

According to latest Drupal Coding Standards, function name should follow lowerCamel convention, so if you can comfortably change so, then do the changes. Else you can keep so if you feel it would hamper your module's work.

google_calendar_merge:

1. MyException is really not a good exception name, use somthing specific to your module a re-use a PHP core exception.
2. "watchdog('_google_calendar_merge_events', $calendar_title . ' ' . $calendar_id . ' found');": this looks vulnerable to XSS exploits. I assume $calendar_title is user provided text and cannot be trusted, so you should use proper placeholders in the watchdog() message with the "%" or "@" placeholder. Make sure to read https://www.drupal.org/node/28984 again. Never concatenate variables into translatable strings for t() or watchdog(), always use placeholders instead.
3. Also looks like your functions are not prefixed with your module name, so you will get name clashes with others.

I haven't tried to exploit the watchdog() security problem, but you should definitely use the correct placeholders in google_calendar_merge, so that is a blocker right now and the reason why we cannot hand out the git vetted user role yet.

But since this project was RTBC already and the only problem was the explanation how it works I think we can at least do a manual promotion of this project. Please get back to us once you need to promote one of the other projects.

Thanks for your contribution, vgriffin!

I promoted this project for you: https://www.drupal.org/project/menu_auto_display

Now that this experimental project has been promoted, you'll need to update the URL of your remote repository or reclone it.

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 stay 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.