[D7] Distill

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

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.

Hey, add please git clone command as it recomended in https://www.drupal.org/node/1011698 (5.4 point)

Automatic Review

Please fix following errors and warnings identified by pareview.

Git default branch is not set, see the documentation on setting a default branch.
Review of the 7.x-1.x branch (commit 497aa8b):

Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
DrupalPractice has found some issues with your code, but could be false positives.
FILE: /var/www/drupal-7-pareview/pareview_temp/distill.module
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
9 | WARNING | There must be no blank line following an inline comment
--------------------------------------------------------------------------------

Time: 65ms; Memory: 5.5Mb

No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.
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.


FILE: /var/www/drupal-7-pareview/pareview_temp/README.md
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 12 WARNINGS AFFECTING 12 LINES
--------------------------------------------------------------------------------
2 | WARNING | Line exceeds 80 characters; contains 172 characters
5 | WARNING | Line exceeds 80 characters; contains 300 characters
8 | WARNING | Line exceeds 80 characters; contains 317 characters
10 | WARNING | Line exceeds 80 characters; contains 383 characters
11 | WARNING | Line exceeds 80 characters; contains 117 characters
12 | WARNING | Line exceeds 80 characters; contains 131 characters
99 | WARNING | Line exceeds 80 characters; contains 450 characters
101 | WARNING | Line exceeds 80 characters; contains 163 characters
104 | WARNING | Line exceeds 80 characters; contains 293 characters
107 | WARNING | Line exceeds 80 characters; contains 298 characters
111 | WARNING | Line exceeds 80 characters; contains 104 characters
113 | WARNING | Line exceeds 80 characters; contains 118 characters
--------------------------------------------------------------------------------

FILE: /var/www/drupal-7-pareview/pareview_temp/distill.module
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
9 | WARNING | There must be no blank line following an inline comment
--------------------------------------------------------------------------------

Time: 144ms; Memory: 7.25Mb

Source: http://pareview.sh/ - PAReview.sh online service

Manual Reviews

- Git clone
Please change git clone url into public
- README.txt/README.md
Please follow the guidelines for in-project documentation and/or the README Template.
- Coding style & Drupal API usage
(*) I can't see any hook system. Can you explain me the reason ?
(*) Remove the packaging lins from the .info. These will be added when the module is published.
(+) Please use t() function for plain text

First of all, great work and great start! Can't wait to see this through to completion!

Git clone instructions on the review page should be used by anyone (currently, it's requiring username patrickocoffeyo). The instructions should be:

git clone --branch 7.x-1.x http://git.drupal.org/sandbox/patrickocoffeyo/2346043.git distill

README.md - please review the module documentation guidelines at https://www.drupal.org/node/161085 and adjust accordingly.

PHP exceptions should use the t function when throwing. See https://www.drupal.org/node/608166 for details.

Your comments can be simplified. For example, in distill_processor.class.inc:

   * @param int $index
   *   Integer representing the delta of the field being processed.
   * @param array $settings
   *   Variable for passing settings and context into processors.

could just be

   * @param int $index
   *   Delta of the field being processed.
   * @param array $settings
   *   Processor configuration.

Also, none of your methods in distill_processor.class.inc have phpdoc for return values.

Document that this module requires PHP 5.3 or higher.

Typo in distill_processor.class.inc

    // If no image stype is specified, return path to full image.

Regarding the feedback about hooks, I almost said ignore it, but actually there's a definite use case; allowing modules that define new field types to create their own processors. Most people could then just use a straight Distill class and only extend in extreme cases. Instead of implementing DistillProcessor as a class, why not a hookable system? The processors you've provided out of the box could then be re-implemented as hooks which will provide additional documentation and examples to others, along with an API.

This is a go. If you need it, you need it bad. Otherwise it's kind of weird.

I can confirm that it works as described, that the code is crisp, clean and well documented, and that it's actually useful.

It's even quite clever :-)

As far as I can tell, the issues raised by previous reviews are all taken care of.

It still lacks a test suite, but I would use it anyway.

Thanks for your contribution!

I updated your account so you can 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 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.