[D7] Joomag filter

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

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.

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 :-)

Hola Mariano,

Please find my findings, all may not be applicable but it's probably interesting to have it:

• Ticket is not following "requirements" - title is missing [D7], git command is missing - we are lazy ;-)
• The repository is containing a .gitignore
• It may be a good idea to rename joomag_filter.inc into joomag_filter.filter.inc, so without opening the file we almost already know what it's containing.
• I don't think it's really useful to have a separate _joomag_filter_information() to return a string which is used only once in joomag_filter_help().
• In _joomag_filter_information(), as per the l() documentation you should use t() and embed the HTML anchor tag directly in the translated string.
• In joomag_filter_filter_info(), it would be better for readability to close the array on a new line:
  

  function joomag_filter_filter_info() {
    $filters['joomag'] = array(
      'title' => t('Joomag filter'),
      'description' => t('For more info visit joomag.com.'),
      'process callback' => '_joomag_filter_filter_joomag_process',
    );
    return $filters;
  }

• In joomag_filter_filter_info(), the joomag.com may be a link.
• Maybe it's because I'm not used to Joomla but I think you should improve functions comment into joomag_filter.inc
• It seems the test file is work in progress, it may be a good idea to move it into a separate git branch during the development.

Thanks,

Multiple Applications

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

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

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

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.

Removing review bonus tag.you have not done manual review you just pasted the output of the phpcbf and phpcs here https://www.drupal.org/node/2426991#comment-9726309 https://www.drupal.org/node/2445345#comment-9725859
Do manual reviews there and if you have not found anything then specify there and set the issue status accordingly.Then Add the review bonus again.

tested and reviewed, no problems were found during the test.
this module does what is said in the description + simple backend.

updated git command.
#13
We usually judge on the basis of the review comment.

Review of the 7.x-1.x branch (commit 71b49a4):

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. Remove the extra 7.x-1.x-tests branch if not needed.
2. joomag_filter_help : Avoid use of html in translatable strings
3. Need to do cleanup in JoomagFilterTests.php Is it needs work ?
4. Readme.txt : Go to "MY ACCOUNT" > "MY MAGAZINES" and select a magazine from the list. I have not found this path because the module is not registering the path.Are you talking about the www.joomag website here. Please add a comment.
5. hook_help is nice.Add a helping text about the regex in this function do _joomag_filter_get_value_with_default

Otherwise looks good to me.

Review of the 7.x-1.x branch (commit 1921f22):

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

1. 1. Sign-in at joomnag.com.Seems type in website address ?
2. Project page needs more information.See the tips https://www.drupal.org/node/997024
3. joomag_filter_help : Avoid use of html in translatable strings

Rest looks good to me.No more blocker.
Assigning to Dave reid to take a final look at this if he has time.

Thanks you for your reviews. If you did not find any problems with an applications please set it to "Reviewed & Tested by the community" after your review.

Review of the 7.x-1.x branch (commit 1921f22):

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

  FILE: /home/klausi/pareview_temp/joomag_filter.module
  ---------------------------------------------------------------------------
  FOUND 3 ERRORS AFFECTING 3 LINES
  ---------------------------------------------------------------------------
   18 | ERROR | [x] Closing parenthesis of array declaration must be on a
      |       |     new line
   31 | ERROR | [x] Closing parenthesis of array declaration must be on a
      |       |     new line
   32 | ERROR | [x] Closing parenthesis of array declaration must be on a
      |       |     new line
  ---------------------------------------------------------------------------
  PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
  ---------------------------------------------------------------------------
  

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

manual review:

• I looked at the regex replacement code and wondered whether an attacker could inject some XSS into the magazine URL or other parts, but could not come up with a working exploit if a limited text format such as filtered HTML is used.

Otherwise looks good to me, so ...

Thanks for your contribution, mariano.barcia!

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.