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%.
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | features-coder_cleanup-2142257-4.patch | 3.91 KB | zhuber |
| #1 | features-coder_cleanup-2142257.patch | 4.16 KB | zhuber |
Comments
Comment #1
zhuber commentedHere is the patch.
Comment #2
zhuber commentedIf 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!
Comment #3
hefox commented> + 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.)
Comment #4
zhuber commentedI'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:
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.
Comment #5
hefox commentedIf you emailed me, I'd likely deliberately not attribute... I'm sorta particular about what email I like receive. Some maintainers attribute, some not.
Comment #6
zhuber commentedI 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.