Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Background: http://drupal.org/node/537828
Here is the patch for updating the Statistics module's Help function. Rewrites.
Comment | File | Size | Author |
---|---|---|---|
#11 | statistics_module_4.patch | 4.61 KB | batigolix |
#5 | help_statistics3.patch | 8.83 KB | arianek |
#5 | help_statistics3.png | 132.01 KB | arianek |
#4 | statistics_module_2.patch | 8.14 KB | batigolix |
#1 | statistics_module.patch | 8.18 KB | batigolix |
Comments
Comment #1
batigolixnew patch with missing closing tag
Comment #3
jhodgdonLooks fairly good, aside from the patch not applying... and I have some comments about the patch:
a)
return $output;
should be indented same as the other lines above it.
b) The other help pages begin the About section with "The xyz module ...". This one should start that way too.
c) "who it viewed, from where it was visited (referrer URL) and when it was viewed" would be clearer as:
"who viewed it, the previous page the user visited (referrer URL), and when it was viewed"
Also note that there needs to be a comma before the "and". See http://drupal.org/about/authoring
d) "The module offers four reports." before the UL, should end in : not .
e) "block that displays today\'s and all time\'s the most viewed pages," -- maybe omit "the" there? or rewrite as
"block that displays the most viewed pages today and for all time"
Comment #4
batigolixgreat. thank you for the review
here's my new attempt (with all your suggestions applied)
Comment #5
arianek CreditAttribution: arianek commentedcleaned up the language a little more and moved the managing logs info into uses - i think this is ready to go...
Comment #6
webchickThis looks great!
I found one bug, which is "To enable the statistics the option" has an extra "the" that it shouldn't. I removed it, but then realized the sentence didn't really make sense as is, so conferred with ariane in IRC and changed this bit to "To enable collection of statistics". This still is a bit awkward (probably should be in active voice rather than passive), but I think this works unless someone comes up with something better.
Committed to HEAD!
Comment #7
batigolixmark that my first contribution to drupal. ever
been only using it for 5 yrs ;)
Comment #8
webchickRIGHT ON!! Welcome to the team, batigolix! :D
Comment #9
arianek CreditAttribution: arianek commentedcongrats batigolix! you've been a huge help this last week, keep up the great work, we really appreciate it! :-)
Comment #10
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.
Needs a quick repatch unless we change the standard.
Comment #11
batigolixhere ye go
Comment #13
lisarex CreditAttribution: lisarex commentedActually consensus was reached that the caps should remain... see #537828: Help text for core modules - update to conform to new standard. So it's actually OK and commit can remain.
Comment #14
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... also something happened with this one in testing...
Comment #15
batigolixin that case the issue can be marked "fixed" as the current version in head *has* the correct capitalization