Closed (fixed)
Project:
Drupal core
Version:
7.x-dev
Component:
documentation
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
16 Nov 2009 at 08:47 UTC
Updated:
14 May 2010 at 16:23 UTC
Jump to comment: Most recent file
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 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 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 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