I ran this module through coder review and it had a few really minor suggestions. I've cleaned them up and now coder passes 100%.

Comments

zhuber’s picture

Status: Needs work » Needs review
StatusFileSize
new4.16 KB

Here is the patch.

zhuber’s picture

Assigned: zhuber » Unassigned

If this patch is committed, please attribute correctly. (http://www.bywombats.com/blog/09-06-2011/giving-credit-where-credit-due)

Here is my author info:
--author="zhuber <zhuber@1437276.no-reply.drupal.org>"

Thanks!

hefox’s picture

Status: Needs review » Needs work

> + drupal_set_message(check_plain($message), $severity, FALSE);

check_plain is only needed if message is from user input, which I doubt this is.

(IMO making a comment specially asking to be git attributed.. is odd.)

zhuber’s picture

Status: Needs work » Needs review
StatusFileSize
new3.91 KB

I've removed the check_plain() frunction from the drupal_set_message call. Coder does not give a 100% pass now, however.

Here is the message:

Line 1018: Potential problem: drupal_set_message() only accepts filtered text, be sure to use check_plain(), filter_xss() or similar to ensure your $variable is fully sanitized. (Drupal Docs) [security_dsm]
drupal_set_message($message, $severity, FALSE);

Also, I agree that it is odd to ask for credit in the comments of a ticket. The reason I've begun to do it is because I've very rarely been attributed a commit without having to email the maintainers. For example, I've had this happen with 3 commits this week alone. After spending time sending the same e-mail over and over to maintainers and never getting the commit amended correctly, I've decided to simply attach it to the ticket with the patch.

hefox’s picture

Status: Needs review » Fixed

If you emailed me, I'd likely deliberately not attribute... I'm sorta particular about what email I like receive. Some maintainers attribute, some not.

zhuber’s picture

I realize that my last comment may have been misleading. I only emailed module maintainers after my changes were committed without attribution. The purpose of said e-mails was solely to amend the commit so it included credit to the author. I never posted these sort of attribution comments before, but I got really tired of trying to get credit for my work after every patch I made. It's not intentional on the maintainer's part, but it does get frustrating and I wanted to save everyone unnecessary time and effort.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.