[D7] Image Source Formatter

Hello carlagno,

Please run automated review and check the errors.
- Git default branch is not set
- Your README.md does not follow best practices
Here is the report. https://pareview.sh/pareview/https-git.drupal.org-project-image_source_f...

Thanks

Thank you for applying! I added the Git instructions for non-maintainer users and the PAReview checklist link. Reviewers will check the project and post comments to list what should be changed.

If you haven't done it, yet, please check the PAReview report and fix what needs to be fixed. There could be some false positives; verify that what reported is correct, before making any change.

#7 was the reason to put the issue back in status "needs work".

If the status is changed to Needs work, the OP should get a description of what needs to be changed, not even a comment saying this link shows what need to be fixed.

PAReview reports just a few errors that for me aren't application-blockers.
@kiamlaluno can we approve the application after a manual review?

@montesajudy It depends on what the review reports.

Thanks for your contribution!

* image_source_formatter_field_formatter_view(): do not use #markup and theme(), just directly return a render array. That way other modules and preprocessors can change stuff more easily.
* image_source_formatter_field_formatter_view(): this is vulnerable to XSS exploits. If I enter a'><script>alert('XSS');</script> as image alt text then I will get a nasty JavaScript popup. You need to sanitize all user provided text before printing to HTML. See https://www.drupal.org/docs/7/security/writing-secure-code/handle-text-i... . And please don't remove the security tag, we keep this for statistics and to show examples of security problems.

Hi there!

Most recent PAReview still shows some issues with README and a few other file errors

Review of the 7.x-1.x branch (commit 0dc8465):

• Your README.txt does not follow best practices (headings need to be uppercase). See https://www.drupal.org/node/2181737 .
  • The REQUIREMENTS section is missing.
  • The CONFIGURATION section is missing.
• The image_source_formatter.module does not implement hook_help(). See https://www.drupal.org/docs/develop/documenting-your-project/module-docu... .
• Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
  

  
  FILE: ...vendor/drupal/pareviewsh/pareview_temp/image_source_formatter.module
  --------------------------------------------------------------------------
  FOUND 2 ERRORS AFFECTING 2 LINES
  --------------------------------------------------------------------------
   102 | ERROR | [x] Array closing indentation error, expected 4 spaces but
       |       |     found 5
   103 | ERROR | [x] Whitespace found at end of line
  --------------------------------------------------------------------------
  PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
  --------------------------------------------------------------------------
  
  Time: 493ms; Memory: 4Mb
  

• DrupalPractice has found some issues with your code, but could be false positives.
  

  
  FILE: ...vendor/drupal/pareviewsh/pareview_temp/image_source_formatter.module
  --------------------------------------------------------------------------
  FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 1 LINE
  --------------------------------------------------------------------------
   37 | WARNING | Unused variable $include_empty.
   37 | WARNING | Unused variable $output.
  --------------------------------------------------------------------------
  
  Time: 204ms; Memory: 4Mb
  

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

*Manual Review*

The method in which you are calling image_style_options is a little bit weird, and that's leading to the "unused variable" message above. https://git.drupalcode.org/project/image_source_formatter/blob/7.x-1.x/i...

I would instead just pass the parameters directly instead of assigning them to variables as you currently are.

I see you added check_plain around your inputs, great! But some are selects and don't accept user input -- do they need to be sanitized as well? https://git.drupalcode.org/project/image_source_formatter/blob/7.x-1.x/i...

From my cursory examination, that's all I could identify. This seems like a useful module though!

Remember to change status, if all the reported issues have been fixed.

I think this needs review now again.

Reviewed this again, looks good to me!

Thank you for your contribution! I am going to update your account.

These 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 more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, 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.

I thank all the dedicated reviewers as well.

@carlagno You are welcome!

;)