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)

Files: 
CommentFileSizeAuthor
#22 taxonomy_check_vocabulary_hierarchy_patch5-1196102-5114164.patch1.47 KBnoisetteprod
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]
#15 taxonomy_check_vocabulary_hierarchy_patch5-1196102-4754366.patch1.47 KBJames_Stallings
PASSED: [[SimpleTest]]: [MySQL] 33,577 pass(es).
[ View ]
#11 taxonomy_check_vocabulary_hierarchy_patch4-1196102-4749410.patch1.48 KBJames_Stallings
PASSED: [[SimpleTest]]: [MySQL] 33,564 pass(es).
[ View ]
#8 taxonomy_check_vocabulary_hierarchy_patch3-1196102-4740768.patch1.49 KBJames_Stallings
PASSED: [[SimpleTest]]: [MySQL] 33,556 pass(es).
[ View ]
#4 taxonomy_check_vocabulary_hierarchy_patch3-1196102-4637986.patch593 bytesJames_Stallings
PASSED: [[SimpleTest]]: [MySQL] 33,547 pass(es).
[ View ]
#3 taxonomy_check_vocabulary_hierarchy_patch2-1196102-4637986.patch593 bytesJames_Stallings
PASSED: [[SimpleTest]]: [MySQL] 33,538 pass(es).
[ View ]
#2 taxonomy_check_vocabulary_hierarchy_patch1-1196102-4637986.patch593 bytesJames_Stallings
PASSED: [[SimpleTest]]: [MySQL] 33,556 pass(es).
[ View ]

Comments

Title:Documentation problem with taxonomy_check_vocabulary_hierarchytaxonomy_check_vocabulary_hierarchy doc lacks return value
Version:7.2» 8.x-dev
Issue tags:+needs backport to D7

Thanks for reporting. 8.x, then backport to 7.x

Status:Active» Needs review
StatusFileSize
new593 bytes
PASSED: [[SimpleTest]]: [MySQL] 33,556 pass(es).
[ View ]

StatusFileSize
new593 bytes
PASSED: [[SimpleTest]]: [MySQL] 33,538 pass(es).
[ View ]

Text editor blew up the spacing in previous patch

StatusFileSize
new593 bytes
PASSED: [[SimpleTest]]: [MySQL] 33,547 pass(es).
[ View ]

Let's try this again..

Status:Needs review» Needs work

This 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.)

Sounds good. I'll make those adjustments.

Assigned:Unassigned» James_Stallings

Assigned:James_Stallings» Unassigned
StatusFileSize
new1.49 KB
PASSED: [[SimpleTest]]: [MySQL] 33,556 pass(es).
[ View ]

I modified the description further. The sentence describing the first return condition didn't make sense.

Status:Needs work» Needs review

Status:Needs review» Needs work

Hmmm... 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"?

Status:Needs work» Needs review
StatusFileSize
new1.48 KB
PASSED: [[SimpleTest]]: [MySQL] 33,564 pass(es).
[ View ]

Thanks, 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.

Status:Needs review» Needs work

To 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!

Assigned:Unassigned» James_Stallings
Status:Needs work» Active

Status:Active» Needs work

status changed back

Status:Needs work» Needs review
StatusFileSize
new1.47 KB
PASSED: [[SimpleTest]]: [MySQL] 33,577 pass(es).
[ View ]

Issue tags:+needs backport to D6

Thanks! 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).

Status:Needs review» Reviewed & tested by the community

Version:8.x-dev» 6.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)
Issue tags:-needs backport to D7

Committed and pushed to 8.x and 7.x. Thanks!

Marking to be ported for 6.x.

Assigned:James_Stallings» Unassigned

Issue tags:+Novice

The backport to D6 should be a good novice project.

Status:Patch (to be ported)» Needs review
StatusFileSize
new1.47 KB
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]

Status:Needs review» Reviewed & tested by the community

Looks good, thanks!

Status:Reviewed & tested by the community» Fixed

Thanks, committed, pushed.

Automatically closed -- issue fixed for 2 weeks with no activity.