[D7] Coffee Extras

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

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.

Manual Reviews:

Please consider security issue. It is not good that use "!" markup for string varible in t() function as security issue.

 $commands[] = array(
      'value' => 'admin/structure/types/manage/' . $content_type->type,
      'label' => t('Manage !type', array('!type' => $content_type->name)),
      'command' => $command . ' ' . drupal_strtolower($content_type->type),
    );

Hi mrkdboyd.

I've found some minor caching issues.

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.

You might want to consider adding Views, Bean and Features to the recommended modules list.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements.

Coding style & Drupal API usage

I noticed, that you have to flush the cache, in order to

1. get renamed features in the commend menu.
2. get renamed vocabularies in the commend menu.
3. get new roles in the commend menu.

This review uses the Project Application Review Template.

Tobi

@mrkdboyd, I had time to review your module again today.

I found some issues in your code.

Manual Reviews

Use (*) and (+) to indicate an issue importance.
(*) Please remove the packaging lins from the .info. These will be added when the module is published.
(*) Please remove t() function in .install file. Please read this article carefully.
https://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/t/7

Agreed with latest comment -- the "package" is used correctly in the .info file currently. http://cgit.drupalcode.org/sandbox-mrkdboyd-2361931/tree/coffee_extras.info

Back to needs review.

I use the coffee module daily and your features look really great but as a first impression a question arises:

Why did you tried to create a new module why not try to help out the current maintainer, eventually join forces?

Here is a helpful documentation https://www.drupal.org/node/23789

This sounds like a feature that should live in the existing coffe project. Module duplication and fragmentation is a huge problem on drupal.org and we prefer collaboration over competition. Please open an issue in the coffee issue queue to discuss what you need. You should also get in contact with the maintainer(s) to offer your help to move the project forward. If you cannot reach the maintainer(s) please follow the abandoned project process.

If that fails for whatever reason please get back to us and set this back to "needs review".

Individual user account
Yes

No duplication
Yes

Master Branch
Yes

Licensing
Yes

3rd party assets/code
Yes.

README.txt/README.md
Well done. Every readme.txt should be like this.

Code long/complex enough for review
Yes

Secure code
Yes

Coding style & Drupal API usage
Very good, very tight code. Nitpick/suggestions:

1. On line 28 you have the following code:

$module_add_commands_function = "coffee_extras_add_{$module}_commands";

Here you're using the inline method to escape variable expressions from strings. But supposedly the more secure and proper way to do it is to use call_user_func(). I've never done it myself, but in looking it up, that seemed to be the recommended most secure method. Here's the documentation for call_user_func();

2. Perhaps add a comment to explain why you're flushing the caches, starting on line 147 – which I assume is to make coffee aware of any changes the user makes to the modules invoked.

Multiple Applications

It appears that there have been multiple project applications opened under your username:

Project 1: https://www.drupal.org/node/2476919

Project 2: https://www.drupal.org/node/2362563

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

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