Clean up API docs for image module

I took a quick glance through this patch, and noticed a few things:

a) We don't actually want @return docs on page callbacks or form generating functions. They are implied, unless something really odd is going on.

Something that is documented as both a page callback and form generating function therefore doesn't really need:

+ * @return array
+ *   A form array which will be rendered into a page via drupal_get_form().

b)
Things like this:

- * This form is used universally for editing all image effects. Each effect adds
- * its own custom section to the form by calling the form function specified in
- * hook_image_effects().
+ * This form is used universally for specifying all image effects. Each image
+ * effect adds its own custom section to the form by calling a form function
+ * specified in hook_image_effects().

("effect" -> "image effect") are not really necessary/welcome in this cleanup. We're just trying to do the things in the list on the meta-issue, not make all the docs perfect. Feel free to file new issues for other problems you find.

c) See http://drupal.org/node/1354#themeable for standards on how to document functions like theme_image_style_list(). In particular, the first line isn't right, and it's not necessary to say it's "the default method of theming" something -- that's implied by the name, and if we added this to all theme functions and templates, that would get very tedious. (also that second sentence about it being overridable)

d) Please do not add data types to param/return values. That's a separate effort, which has yet to be organized.

e) Since you filed a separate issue to clean up list formatting, let's not do that here either (and it's not in the list of things to clean up in the meta-issue). Or file a separate issue for that.

f) Hook definitions do not have the same verb requirements as regular functions:
http://drupal.org/node/1354#hooks

g) Refer to modules as "the Sentence case human-redable name" or "machine_name.module", not:

/**
  * @file
- * Functions needed to execute image effects provided by Image module.
+ * Functions needed to execute image effects provided by image module.
  */

h) No code changes here, only docs:

   if (!image_resize($image, $data['width'], $data['height'])) {
-    watchdog('image', 'Image resize failed using the %toolkit toolkit on %path (%mimetype, %dimensions)', array('%toolkit' => $image->toolkit, '%path' => $image->source, '%mimetype' => $image->info['mime_type'], '%dimensions' => $image->info['width'] . 'x' . $image->info['height']), WATCHDOG_ERROR);
+    $variables = array(
+      '%toolkit' => $image->toolkit,
+      '%path' => $image->source,
+      '%mimetype' => $image->info['mime_type'],
+      '%dimensions' => $image->info['width'] . 'x' . $image->info['height']
+    );
+    watchdog('image', 'Image resize failed using the %toolkit toolkit on %path (%mimetype, %dimensions)', $variables, WATCHDOG_ERROR);
     return FALSE;

Once these are cleaned up, I'll give it a more thorough review. Thanks!

There are a couple places where parameters weren't documented and I couldn't tell what they were:
image.install: image_requirements()
image.module: image_filter_keyword()

I will make issues for these. It seems like I should wait until this issue is closed before I do that to avoid conflicts.

Sending to the bot.

#5: image-clean-up-documentation-1326608-5.patch queued for re-testing.

Sorry this took so long to get reviewed...

In the meantime, we had a discussion on #1315992: No standard for documenting menu callbacks and we have (unfortunately) changed the standard for hook_menu() callbacks:
http://drupal.org/node/1354#menu-callback

So this patch will need an update.

Tagging.

Also @xenophyle: are you still planning to work on this? If so, please respond. If not, we'll unassign it so someone else can. Thanks!

i'm slowly working on this

sorry for this taking such a time but have been having some problems.

proper feedback is welcome of course

hi

if anyone please can take some time to 24,25/5 tomake a review of this then i'll have time todo a new patch if needed

Sorry, there are quite a few cleanup issues waiting for review right now... I am doing them about one a day. Yours will be reviewed sometime...

+++ b/core/modules/image/image.admin.inc
@@ -26,11 +26,11 @@ function image_style_list() {
  *   An image style array.
+ * ¶
  * @ingroup forms
+ * ¶

A lot of trailing white-spaces in patch. Also there's a lot of @todo waiting for #1464554: Upgrade path for image style configuration

@jhodgdon due to accident and operations etc etc I haven't have time with this and I know my reply to u is way late but just so you know; My request/wish was not targeting you (I do know how much you have to do) but generally to anyone reading the topic.

sincerely Peter

Rerolled the patch. Now going further with cleanup

Further cleanup of whitespaces and indention.
Left is param and function return types. But this was out of scope for this issue. I'm not sure if i should fix array 80 character overflows.
I didn't do this for now.

Thanks! This patch needs some additional cleanup:

a) Lines like this:

* Form builder; Edit an image style name and effects order.

The standard for form-generating functions is at http://drupal.org/node/1354#forms -- also note we don't need a blank line between the @ingroup and the @see at the end, and the @ingroup should be last.

b)

t('If this style is in use on the site, you may select another style to replace it. All images that have been generated for this style will be permanently deleted.'),
-    t('Delete'),  t('Cancel')
+    t('Delete'), t('Cancel')
   );
 }

This issue is limited to documentation changes. Do not make code line changes at all in this issue. Those are being addressed on other issues. See #1310084: [meta] API documentation cleanup sprint for the scope of this issue.

c)

+ * Element validate handler to ensure that either a height or a width is set.
  */
 function image_effect_scale_validate($element, &$form_state) {

Standards for this: http://drupal.org/node/1354#render

d)

/**
- * Pull in image effects exposed by modules implementing hook_image_effect_info().
+ * Pull image effects exposed by modules implementing hook_image_effect_info().

Verb tense -- see http://drupal.org/node/1354#functions -- check the rest of the functions in this directory for the same problem (there are others in the patch with wrong verb tense).

e)

* @return
  *   An array containing the image effect definition with the following keys:
- *   - "effect": The unique name for the effect being performed. Usually prefixed
- *     with the name of the module providing the effect.
+ *   - "effect": The unique name for the effect being performed. Usually
+ *     prefixed with the name of the module providing the effect.
  *   - "module": The module providing the effect.
  *   - "help": A description of the effect.
  *   - "function": The name of the function that will execute the effect.

Could clean up the list syntax: http://drupal.org/node/1354#lists

f)

* @param $value
+ *   The keyword for the filter
  * @param $current_pixels
+ *   The current pixel
  * @param $new_pixels
+ *   The pixel offset
+ *
+ * $return
+ *  The new pixel offset
  */

Formatting -- needs . at end of lines -- see http://drupal.org/node/1354#functions

Rerolled #20.

Looks good to go

I don't think the comments in #22 have been addressed at all.

They haven't. I only rerolled the last patch. There was enough to do just creating the reroll. I hope to get to the changes in #22 later this evening.

This needs full re-work because of dramatic changes of image module during this summer.

These issues are a lot of work with very little tangible payoff, so I'm closing the rest of them as "won't fix". Your efforts on working on this issue were appreciated... it was just my fault for starting a task that was very difficult to get right.

Let's instead put our effort into fixing and reviewing documentation that is really unclear and/or wrong, and I hope that the people who worked on these issues are not afraid to jump into a more reasonable issue!