user_validate_picture is undocumented; user_account_form() validation functions missing @see links

@aaronmontana: Why you have assigned it to yourself?

I thought you were supposed to assign them to yourself if you were working on them. Wrong? Let me know what the protocol is I'm a little new to the docs.

Also, the reason I haven't posted the patch yet is that I worked on another little doc in the same file, #764426: user_validate_mail is undocumented , which is still pending. I can't figure out how to generate a patch that doesn't have both changes. I'm new to CVS as well, more of an SVN guy. I've attached the patch... any tips?

Thanks again - you can grab this one if you want to its a very simple function doc.

Subscribe

I see someone else has joined the discussion - I had a couple more questions.

1) This is a very simple function and the description I added is basically one line, with very short versions the descriptions for $form and $form_state. Should it be more though? I toyed with describing it in more detail, but it seemed like it just repeated documentation found else where. Thoughts?

2) Looking at more documentation I noticed some inconsistency with params that are passed by reference. Should it be explained and shown, or do we maybe explain this with inline comments in the code itself?

Any help would be great, I'd really like to contribute more documentation. thanks! :)

Subscribe

a) Assigning it to yourself when you want to work on it is the right procedure. You can put a little note in like "I plan to do this in the next 3 days" so people have some idea of the time frame, in case you never get back to it.

b) You don't have to add a "subscribe" in order to subscribe, if you have already been commenting -- any comment on an issue automatically subscribes you. However, it doesn't cause any notifications (like email messages on update). To get those, you need to go to http://drupal.org/project/issues/drupal and click on "subscribe" to set your notification preferences.

c) Each patch needs to have just the changes for the issue you attach it to. You need to revert the changes from the other issue before you start working on this issue, and vice versa. You won't lose your work though, because you can always apply the patch you created again. :)

Regarding the patch itself:
For both of these functions, you need a @return section explaining what the return value is.

a) Thanks, thought maybe I was being rude.

b) Silly me.

c) Total 'duh' moment for me.

On the return value - user_validate_picture() doesn't explicitly return a value, but it does makes some changes to $form_state, since it is passed by reference. I have made changes to the comment to reflect this. I didn't find another example of this elsewhere, (admittedly I didn't look that hard) but I think it is pretty clear in my update to the doc. Let me know if that was the right approach. thanks again.

You need to set the issue to "needs review" so that we know it is ready to be reviewed, and to let the test bot run the test on the patch.

Actually, we don't put & in the @param part of the doc. It is just in the function definition.

One other minor problem

validates and uploads; that will be reflected

that ; should be a ,

@aaronmontana:
1. Welcome to the community
2. I hope some existing documentation with similar function definition can easily help you. (I used similar formula to commit my initial patches :) )

Replaced semicolon with comma
Removed ampersands from params

Excellent, thanks!

Thanks for the great work on this! However... (please don't hit me! :)) we don't actually document $form and $form_state, because those are the standard arguments in any form definition function. If we introduced these docs here, we'd have to copy/paste that @param and @return stuff on 100s of other functions, to keep them consistent. Sorry, I know this is confusing, and I think it's the only place where we make this exception outside of the test framework.

Would it be possible to get a re-roll of this that's just the one-liner description instead?

In fact a description of the function is good enough. Normally most of the developers know about $form and $form_state

Actually, this function should also have a @see link to the form it's validating. (Oops, missed that in the review above.) That would be user_account_form()

That function should also have @see lines to point to user_validate_picture(), user_account_form_validate(), user_validate_current_pass() (which are the validation functions for that form), and it would be good if all of these functions also had an @see that points back to user_account_form().

We have some standards on how to document form-related functions, by the way:
http://drupal.org/node/1354#forms

I think this should be right. I get what your saying about $form vars, it would add quite a bit of redundant documentation.

Can you add the other @see links in too? (mentioned in #17)

Sorry for the delay, day job keeping me pretty busy. Okay, so I re-read your last comment a couple times and finally realized you were telling me to go ahead and add the proper comments to the other functions in the same module (right?). I didn't know if it was okay to include those changes in the same patch/issue, since its pretty simple stuff I'm guessing it is. I added @see links for everything touching the user_account_form(), I think this is right based on what I read in the form documenting docs, but just let me know. Also its just one patch, but if need be I can go back break it down into separate parts.

I'm a little slow, but I think I'm starting to get the hang of this, thanks for everyones help!

That's waht I was looking for! Thanks!

One minor problem:

 /**
  * Form validation handler for the current password on the user_account_form().
+ * 
+ * @see user_account_form
  */

That @see needs to have () after user_account_form() in order for it to turn into a link on api.drupal.org.

Whoops. Corrected.

Thanks!

Committed to CVS HEAD. Great work! :)