API page: http://api.drupal.org/api/drupal/modules--taxonomy--taxonomy.module/func...
Describe the problem you have found:
No return value described in the documentation. From the code I would say it's an integer with 0, 1 or 2. Something like:
@return lowest level of hirarchy of the given vocabulary.
Could be added to the docs.
(I see that IN the PhpDoc the 0,1,2 stuff is described, but the RETURN value is not separatly described, which gets used in may IDEs for Intellisense and also in the Drupal Docs it creates an extra H3 tag, which is more visible and eyecatching)
Comments
Comment #1
jhodgdonThanks for reporting. 8.x, then backport to 7.x
Comment #2
James_Stallings CreditAttribution: James_Stallings commentedComment #3
James_Stallings CreditAttribution: James_Stallings commentedText editor blew up the spacing in previous patch
Comment #4
James_Stallings CreditAttribution: James_Stallings commentedLet's try this again..
Comment #5
jhodgdonThis is almost OK, but:
a) While you're editing this function's doc, you could also update the verb tense in the first line (check -> checks) ... and probably get rid of that word "dynamically", which seems utterly meaningless to me?
b) In the description paragraph:
updates the vocabularies hierarchy
->
vocabulary's hierarchy
c) and:
If any term contain multiple
->
contains
d) You need a blank line between the @param and @return section.
e) In your patch:
of the $vocabulary's hierarchy.
-->
of the vocabulary's hierarchy
or
of $vocabulary's hierarchy
(Either use it as a variable or don't use it as a noun.)
Comment #6
James_Stallings CreditAttribution: James_Stallings commentedSounds good. I'll make those adjustments.
Comment #7
James_Stallings CreditAttribution: James_Stallings commentedComment #8
James_Stallings CreditAttribution: James_Stallings commentedI modified the description further. The sentence describing the first return condition didn't make sense.
Comment #9
James_Stallings CreditAttribution: James_Stallings commentedComment #10
jhodgdonHmmm... I have a few suggestions:
a) There are spaces at the ends of several lines. (Many programming editors have an option to highlight or remove end-of-line text -- you might check and see if you can turn that on in your editor.)
b) If any term contain multiple parents
contain -> contains
c) Actually, I don't like the word "contain" in this paragraph. Terms don't "contain" parents -- if anything, it's the other way around. Probably it would be better to change it back to have/has. This phrase in particular I thought was confusing: "If no term contains a single parent..." -- I don't think it's as clear as the original... How about "If no term has parent terms"?
Comment #11
James_Stallings CreditAttribution: James_Stallings commentedThanks, I've made the recommended changes. BTW is there a way to receive email notifications when a new comment has been added to a post your user account is active on.
Comment #12
jhodgdonTo receive notifications, go to http://drupal.org/project/issues/drupal and click on Subscribe. If you choose "Own issues" you will get email for updates on any issue you either created or have commented on.
So... This patch is still using the word "contains" in a way that I think is confusing:
If any term contains a single parent
If any term contains multiple parents
Could you change those to "has" like you did for the first one? Thanks!
Comment #13
James_Stallings CreditAttribution: James_Stallings commentedComment #14
jhodgdonstatus changed back
Comment #15
James_Stallings CreditAttribution: James_Stallings commentedComment #16
jhodgdonThanks! This version reads well to me. 8.x/7.x please... Then it looks like we should backport to Drupal 6, which has the same problem (and pretty much if not exactly the same function code).
Comment #17
jhodgdonComment #18
webchickCommitted and pushed to 8.x and 7.x. Thanks!
Marking to be ported for 6.x.
Comment #19
James_Stallings CreditAttribution: James_Stallings commentedComment #20
jhodgdonThe backport to D6 should be a good novice project.
Comment #22
noisetteprod CreditAttribution: noisetteprod commentedComment #23
jhodgdonLooks good, thanks!
Comment #24
Gábor HojtsyThanks, committed, pushed.