Attaching minor patch to apply doxygen to function "user_admin" in user.admin.inc, page callback to admin/user/user path.

Took me a couple minutes to figure out what this function does when I was stepping through Drupal's form process and saw this function. You can see there's no doxygen on this function and therefore the API page has no explanation: http://api.drupal.org/user_admin

Files: 
CommentFileSizeAuthor
#24 user_admin_docs-1247812-24.patch952 bytesAlbert Volkman
PASSED: [[SimpleTest]]: [MySQL] 48,905 pass(es).
[ View ]
#22 user_admin_docs-1247812-22.patch597 bytesAlbert Volkman
PASSED: [[SimpleTest]]: [MySQL] 48,948 pass(es).
[ View ]
#19 1247812-user_admin-d8-doxygen.patch931 bytesjzacsh
PASSED: [[SimpleTest]]: [MySQL] 32,911 pass(es).
[ View ]
#16 found_Menu_callbackd8.txt12.89 KBjzacsh
#13 1247812-user_admin-d8-doxygen.patch921 bytesjzacsh
PASSED: [[SimpleTest]]: [MySQL] 32,917 pass(es).
[ View ]
#11 1247812-user_admin-d6-doxygen.patch895 bytesjzacsh
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]
#11 1247812-user_admin-d7-doxygen.patch895 bytesjzacsh
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]
#11 1247812-user_admin-d8-doxygen.patch895 bytesjzacsh
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]
#3 1247812-user_admin-doxygen-d6.patch895 bytesjzacsh
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1247812-user_admin-doxygen-d6.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#3 1247812-user_admin-doxygen-d7.patch895 bytesjzacsh
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1247812-user_admin-doxygen-d7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#3 1247812-user_admin-doxygen-d8.patch895 bytesjzacsh
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1247812-user_admin-doxygen-d8.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#2 1247812-fix-user_admin-doxygen-for-d7.patch772 bytesjzacsh
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1247812-fix-user_admin-doxygen-for-d7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#2 1247812-fix-user_admin-doxygen-for-d8.patch772 bytesjzacsh
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1247812-fix-user_admin-doxygen-for-d8.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#1 1247812-fix-user_admin-doxygen-for-d6.patch765 bytesjzacsh
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1247812-fix-user_admin-doxygen-for-d6.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

StatusFileSize
new765 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1247812-fix-user_admin-doxygen-for-d6.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Attached for 6.x

StatusFileSize
new772 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1247812-fix-user_admin-doxygen-for-d8.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
new772 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1247812-fix-user_admin-doxygen-for-d7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Attached are d7 and d8 patches as well.

StatusFileSize
new895 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1247812-user_admin-doxygen-d8.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
new895 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1247812-user_admin-doxygen-d7.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
new895 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1247812-user_admin-doxygen-d6.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Actually, here's the same patches for d6, d7, d8 with @param used, as an attempt to explain the strangeness going on in this function's $op.

Looks good to me! Cleanly applied to all three versions.

Documentation patches rock!

Status:Needs review» Reviewed & tested by the community

Looks good to me, applied cleanly to each version of Drupal.

Component:user.module» documentation

Fixing "component", per @jhodgdon

Version:6.x-dev» 8.x-dev
Component:documentation» user.module
Status:Reviewed & tested by the community» Needs work

Thanks for the patch!

This doesn't follow the documentation guidelines however, and in English we don't put a capital letter after a semicolon (;). So it needs to be fixed.

Please read
http://drupal.org/node/1354 for doc standards, and attach a patch for Drupal 8 only first, then backport to d7/d6. Thanks!

Component:user.module» documentation
Issue tags:+needs backport to D6, +needs backport to D7

cross-post and adding tags

Also, to the reviewers above - please do not mark patches RTBC unless you have verified that they follow the doc standards, and the test bot has come back green. Thanks!

Version:8.x-dev» 6.x-dev
Status:Needs work» Reviewed & tested by the community
Issue tags:-needs backport to D6, -needs backport to D7
StatusFileSize
new895 bytes
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]
new895 bytes
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]
new895 bytes
PASSED: [[SimpleTest]]: [MySQL] 190 pass(es).
[ View ]

Correcting name schema for patch files so test bot doesn't ignore them.

Version:6.x-dev» 8.x-dev
Status:Reviewed & tested by the community» Needs work
Issue tags:+needs backport to D6, +needs backport to D7

tagging and fixing cross-post again

StatusFileSize
new921 bytes
PASSED: [[SimpleTest]]: [MySQL] 32,917 pass(es).
[ View ]

Fixed doxygen style per #8 - http://drupal.org/node/1354 is very useful documentation!

Status:Needs work» Needs review

Status:Needs review» Needs work

Please read: http://drupal.org/node/1354#function
Function doc first line should start with a verb ending in s (third person).
Param should start with capital letter (after the optional).

Also, maybe it would be helpful to say what the flags are in the param? Normally flag means a constant... these are not constants are they?

StatusFileSize
new12.89 KB

Do we really want the "verb first"-rule applied in this case? Everywhere else in the codebase we have, "Menu callback; [verb]s" written. If everyone agrees that even menu callbacks should conform to this then that's a different story and maybe an issue should just be opened to change all of them in core in one shot.

As for this patch, I personally really like the current (possibly wrong) convention.
You can see a ton of our code still does it: $ grep -rin ' \* Menu callback' modules

@jhdogdon, thoughts?

In your current patch, you have:

Menu callback; generate the appropriate user admin form.

It should at least be:

Menu callback: generates the appropriate user admininstration form.

We had an issue a while back to make standards for documenting menu callbacks, and it was never resolved. Guess we should find it and finish the discussion and get it resolved... Just because everything else in core is also bad is not a good reason to make a bad patch that conforms with the rest of the bad stuff.

Title:patch to apply doxygen to function "user_admin" in user.admin.inc, page callback to admin/user/user pathuser_admin function has no doc

more appropriate title

StatusFileSize
new931 bytes
PASSED: [[SimpleTest]]: [MySQL] 32,911 pass(es).
[ View ]

@jhodgdon Thanks for the tip. I'd be happy to tackle that issue you mentioned, if we find it.

Attached is the fix per comment #17.

Status:Needs work» Needs review

Whoops, setting to needs review.

Status:Needs review» Needs work

The first line is OK I guess, since we don't have standards on this yet.

I just noticed something else though in the @param:

+ *   (optional) flag to indicate which form to build. Defaults to '', which
+ *   will trigger the user filter form.

a) Flag should be capitalized.

b) Is it really a flag? To me, "flag" indicates usually a PHP constant, but in this case it is a string?

Status:Needs work» Needs review
StatusFileSize
new597 bytes
PASSED: [[SimpleTest]]: [MySQL] 48,948 pass(es).
[ View ]

Concerns in #2 addressed. Updated for D8.

Status:Needs review» Needs work

Wow, that's a wacky function! It looks like it can build a multiple user delete confirm form, a user create form, or the default user admin form that lists users (with filtering). It also looks like $_POST['op'] overrides the argument $callback_arg, and I wouldn't say that if $callback_arg is '' that the user filter form is always what is displayed... Really only '' and 'create' are ever passed to $callback_arg, but $_POST['op'] can override it.

So, I think the @param needs some attention and this function needs a @return that says that one of those forms is returned depending on $callback_arg and $_POST values.

Status:Needs work» Needs review
StatusFileSize
new952 bytes
PASSED: [[SimpleTest]]: [MySQL] 48,905 pass(es).
[ View ]

How's this?

Status:Needs review» Reviewed & tested by the community

Thanks, that works!

Status:Reviewed & tested by the community» Fixed

Thanks again -- patch in #24 is committed to 8.x, 7.x, and 6.x (I checked and the 6.x function works the same way as 7/8, and remarkably, the patch applies fine to all three).

Its a magical patch.

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