Tiny code style fixes
Aron Novak - June 23, 2009 - 13:21
| Project: | FeedAPI |
| Version: | 6.x-1.x-dev |
| Component: | Code feedapi (core module) |
| Category: | task |
| Priority: | normal |
| Assigned: | Aron Novak |
| Status: | closed |
Description
Missing @file blocks, wrong styling at dot operator, misuse of db_query in hook_install() and a serious one: feedapi_install() ereased some variables, which is foolness, it should be done in _uninstall(), i assume.
| Attachment | Size |
|---|---|
| feedapi_code_style.patch | 8.79 KB |
#1
Good cleanup. I fixed some minor issues:
- Use imparative instead of third person for comments - i. e. "Create new content type." instead of "Creates new content type."
- @file comments should be more generic in install files - I limited them all to a very generic "Install file for..." which I feel is in line with the comments for hook implementations.
- Cleaned up some language.
- Simplified drush @file comments.
Open question: there are three occasions where regexes have been changed? Was this on purpose? E. g.:
- preg_match_all('/type\s*=\s*("|'. "'" .')([A-Za-z\/+]*)("|'. "'" .')/si', $link, $mime);+ preg_match_all('/type\s*=\s*("|'."'".')([A-Za-z\/+]*)("|'."'".')/si', $link, $mime);
#2
#1: The content of the regexes have not been changed, it's code style question.
"string concatenation should be formatted without a space separating the operators (dot .) and a quote" - coder module
According to this: "." - this is the good :)
However this is changed in these days: http://drupal.org/coding-standards
"Always use a space between the dot and the concatenated parts to improve readability." - as I know this is for D7
However this is a really marginal question.
#3
#2 : stupid me :) haven't read the string properly, didn't notice that these " where actually delimiting the very regex string. But it's weird how this is being used to excape quotes. Shouldn't we use the proper escaple character for this?
#4
Rerolled.
In fact those different style quotes were really amigous :)
#5
One of the regular expressions was skipped from the update.
#6
Ready to be committed.
#7
Committed.
#8
Automatically closed -- issue fixed for 2 weeks with no activity.