I ran coder on 5.x-2.x. This patch also fixes a comment typo. I left the instances of array ( generated by views in project_issue.views.inc since this will hopefully be fixed in views soon. Also didn't change any concatenation issue since the new D7 style could be applied in 6.x once the branch has been created.

CommentFileSizeAuthor
#3 project_issue_code_style.3.patch18.47 KBscor
project_issue_code_style.2.patch17.99 KBscor

Comments

aclight’s picture

aclight commented
Status: Active » Needs review

Thanks for doing this. I haven't tested this yet, but a fairly quick code review shows only one issue.

The following code is really a bug, I believe, and ideally we'd create a separate issue for that so it can be fixed for sure before the next official release. That'd be a trivial patch to review and commit.

-      $last_sid = db_result(db_query('SELECT MAX(sid) FROM project_issue_state'));
+      $last_sid = db_result(db_query('SELECT MAX(sid) FROM {project_issue_state}'));

I'll try to give this patch a thorough review this weekend.

I just added this issue to the list of things to get fixed before the next 5.x release of the module (list at http://groups.drupal.org/node/16069). It might be nice to get some of the other items on that list done first, as if this one gets committed before the rest of them we might need to reroll a bunch of other patches. But either way this should be pretty easy to get committed.

BTW, thanks for not making any string concatenation changes. I have written the D6 port of the project module using the D7 style of concatenation, and so I feel that there's little point to "fixing" any incorrectly written concatenations by D5 standards only to then unfix them for D6 or D7.

Anonymous’s picture

Anonymous (not verified) commented
Priority: Minor » Normal
Status: Needs review » Reviewed & tested by the community

Coding style changes only.

scor’s picture

scor commented
StatusFileSize
new project_issue_code_style.3.patch18.47 KB

missed one true in the previous patch.

dww’s picture

dww
we/he/they
commented
Status: Reviewed & tested by the community » Postponed

This conflicts with #98278: project* namespace bugs in $node and I definitely don't want to have to reroll that patch for these changes. ;) Let's reroll this one after that (and perhaps a few others in the queue) land. That'll be much easier than having to reroll the others, since these are such obvious changes to make. Thanks!