Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Background: http://drupal.org/node/537828
Here is the patch for updating the Image module's Help function. Rewrites.
Comment | File | Size | Author |
---|---|---|---|
#19 | help_image_module_7.patch | 4.56 KB | batigolix |
#16 | help_image_module_6.patch | 4.55 KB | batigolix |
#15 | help_image_module_6.patch | 4.56 KB | batigolix |
#10 | help_image5.patch | 4.71 KB | arianek |
#9 | help_image4.patch | 4.7 KB | arianek |
Comments
Comment #1
batigolixadded #d7help
Comment #2
batigolixnew patch with missing closing tag
Comment #4
jhodgdonReview notes:
- The other help pages all start the About section with a phrase like "The xyz module blah blah blah". This one is different; probably should conform to the other pages' style.
- For ease of translating, please put each paragraph into one t() call. It's very difficult to notice things like "end of sentence. " [the space that is part of the string to be translated) in the translation interface.
- Style/punctuation:
"By combining effects as crop, scale and desaturate effect you can create for example square, grayscale thumbnails."
should be
"By combining effects as crop, scale, and desaturate, you can create, for example, square, grayscale thumbnails."
(Doc style guidelines require the US convention of comma before "and" in a list, and "for example" always needs to be set off by commas before and after when it's used this way.)
Comment #5
batigolixdidnt see where the problem was with your 2nd remark about the paragraph not being in a t function
here is my new attempt
Comment #6
marcvangendjhodgdon didn't mean that there are paragraphs outside t(), he meant that it's better to put a complete paragraph in one single t() instead of breaking it up into several t()'s. By doing that, you avoid having a space as the last character of a translatable string, which is easily overlooked by translators.
Comment #7
arianek CreditAttribution: arianek commentedI sorta overhauled this one - got mucho consultation from @webchick on it, but it still will need a major review. Took out the naming arrays, moved that into ul (note ul/li doesn't show in the screenshot cause of the seven theme). Rewrote lots and added a section on image field. Also added the link to the handbook page, BUT it's super out of date... so not 100% sure it should be included (I just added it for consistency's sake).
Comment #8
arianek CreditAttribution: arianek commentedoh dear, i totally munged the indentation. will re upload.
Comment #9
arianek CreditAttribution: arianek commentedfixed
Comment #10
arianek CreditAttribution: arianek commenteddang, one more fix
Comment #11
batigolixlooks fine to me
i was sceptical about the whole bit about "naming convention" and the theme-list-item method to show a simple two item bullet list, but i didnt heave the guts to remove all of it.
see my remarks in #569184: Helptext Image module is not consistent with example loaded by default
Comment #12
arianek CreditAttribution: arianek commentedyes, it seems it is from way back because quicksketch had actually written extensive documentation for his own module (rather than it being added later by someone else). So for consistency's sake, just moved that all into text to make it easier for translation, consistent, etc.
i wouldn't have done it without "permission" either, but that's what IRC is for! ;-)
so, if anyone else wants to review this go for it, otherwise batigolix, you can probably RTBC.
Comment #13
batigolixset status to RTBC
Comment #14
jhodgdonAgain because of translating, I don't think it's a good idea to have line returns in the middle of text that is being run through t(). Please separate out the HTML tags as much as possible too, and just put the text inside t(), except for inline tags like EM.
Comment #15
batigolixfresh new patch
Comment #16
batigoliximproved patch
Comment #17
arianek CreditAttribution: arianek commentedlooks like a fine patch to me. RTBC timez.
Comment #18
jhodgdonThe standard for capitalization is not being followed. See http://drupal.org/node/632280 -- should start with "The foo module" not "The Foo module" according to standard.
Comment #19
batigolixthere comes another one
Comment #20
batigolixneeds review
Comment #21
jhodgdonStandard for capitalization was changed, see http://drupal.org/node/537828#comment-2303764 and http://drupal.org/node/632280 -- we are now saying the name of the module should be capitalized. Sorry for the inconvenience but we need a new patch...
In the latest one, at some point it says "Image module" without "the" in front of it, which seems wrong to me. Anyway it is close..
Comment #22
arianek CreditAttribution: arianek commentedWEBCHICK!!! pls use patch in #16 (without the caps change) http://drupal.org/files/issues/help_image_module_6_0.patch
Comment #23
webchickJust a minor tweak:
"...and to attach images to content using an image field."
"... and provides an Image field for attaching images to content."
This seemed more in-line with the earlier stuff in that sentence.
Committed #16 to HEAD!