Closed (fixed)
Project:
Project issue tracking
Version:
7.x-2.x-dev
Component:
Comments
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
10 May 2012 at 23:32 UTC
Updated:
4 Jan 2014 at 02:05 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
senpai commentedAssigning and tagging.
Comment #2
iamcarrico commentedAssigning
Comment #3
iamcarrico commentedOkay, here I moved the setting over, and changed all functions that related to it in its previous life.
Comment #4
dwwA) Can you fix the code style bugs in here? I could of course add the missing spaces myself, but if I keep asking you about this, I'm hoping you'll just get in the habit of writing the code to conform to Drupal's code standards in the first place. ;)
B) Have you confirmed that D7's project_issue_preprocess_comment() works the same as D6, and that this actually works as expected?
C) Seems unnecessary to append to variables[] in the first
foreachinproject_issue_uninstall(), just so we can iterate over each of those again a few lines later. Wouldn't it be clearer (and a bit more efficient) to just callvariable_del()directly while iterating over the node types (and move that code block to after where we iterate over$variables)? That'd also make more sense once we have multiple per-issue-node-type settings and we need a nestedforeach. See what I mean?Thanks,
-Derek
Comment #5
senpai commentedComment #6
iamcarrico commentedA) Done, I also fixed a few bugs across the module as a whole.
B) It does.
C) I was merely using what was already there. Personally, I dont see the point of the for_each in the first place, but that is just me. (Unless we make a foreach based on a query on the variable table for anything starting with "project_issue%", which would remove most of the excess code there. But as you say, so shall it be done.
Comment #7
dwwA) Cool, although I split off the code-style-only changes to a separate commit so as not to confuse things. Also, you still had this:
foreach($nodetypes as $name => $obj) {instead of this:foreach ($nodetypes as $name => $obj) {(note the space afterforeach). ;) Fixed that.B) Indeed! Also tried it myself and it's working like a charm. ;)
C) Yeah, it's minor crap, I admit. I should probably stop caring at that level of detail. Thanks for humoring me and my ridiculously high standards.
Committed and pushed... yay!
Thanks,
-Derek
Comment #8
iamcarrico commented