Closed (fixed)
Project:
Drupal core
Version:
9.1.x-dev
Component:
documentation
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
22 May 2020 at 06:08 UTC
Updated:
9 Jul 2020 at 14:09 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
sja112 commentedComment #3
sja112 commentedComment #4
kunalgautam commented#3 patch looks good to me, I checked with grep -r "cotrol" * command and there is no other word after applying the patch.
Good to move to RTBC
Comment #5
ketikagrover commentedShows empty. Moving this to RTBC.
Thanks!
Comment #6
xjmNow that #2972224: Add .cspell.json to automate spellchecking in Drupal core is in, we can make sure this spelling error never happens again by removing the entry from Drupal's dictionrary, so let's update the patch to include that. Thanks!
Comment #7
jungleRemoved it from the dictionary, and fixed "over 80 chars".
Comment #8
jungleAs the changes in #7 is small/tiny, setting back to RTBC.
Comment #9
xjmAh oops, the one-line summary shouldn't be over 80 characters, but it also shouldn't be wrapped onto a second line. It's always tricky when we fix one documentation standards bug in a line that has other documentation standards bugs.
In this case, let's expand the scope of this issue slightly to fix it. I think we can just get rid of the words "Helper function to..." and just say (e.g.):
That should hopefully fit.
Thanks!
Comment #10
mohrerao commented@xjm, I found many instances of single line comments with character length > 80 in the same file core/modules/user/tests/src/Functional/UserPasswordResetTest.php
Should we fix all of them? Otherwise only one fixed comment would break the pattern followed.
Edit: Also there are many lines of code which exceed 80 character length.
Adding a patch for the same in sometime
Comment #11
jungleNice catch! Did not notice it's in function/method's docblock.
@mohrerao, No, just fix what we touched. re-wording/adjust them to one line.
Comment #12
jungleAs suggested, remove "Helper function to..." would be enough, -- less than 80 chars, and all in one line.
Comment #13
mohrerao commentedAdding a patch fixing one-line summary.
Comment #14
jungleThanks, @mohrerao! The extra two changes are fine. Testing won't fail, touched comments and the dictionary file only. So setting back to RTBC directly.
One more thing, a patch for 9.0.x is expected.
Comment #15
jungleWell, upload a patch for 9.0.x
Comment #16
mohrerao commentedAdding a patch for 9.0.x
Comment #17
jungleFunny, both #15 and #16 are wrong.
Comment #21
xjmYeah, given the scope expansion, I think it's okay to also fix those other two that were basically the same header, just with the word spelled correctly.
The many other one-line summaries over 80 characters or wrapped to two lines need to be handled with our coding standards process. All our coding standards issues should be done on a per-rule basis across all of core, rather than fixing standards in individual modules or files. We should also separate fixes where we need to write new documentation from fixes where we need to correct existing standards. This all should be done as part of #2571965: [meta] Fix PHP coding standards in core, stage 1. A good place to start is the child issues of #2572645: [Meta] Fix 'Drupal.Commenting.FunctionComment' coding standard.
For background information on why we usually will not commit coding standards fixes that aren't scoped in that way, see the core issue scope guidelines, especially the note about coding standards cleanups. That document also includes numerous suggestions for scoping issues including documentation coding standards cleanups.
One note for @ketikagrover for future reference -- be sure to use
grep -ri. The dictionary is case-insensitive, so our search needs to be as well.Committed #13 to 9.1.x. Committed #17 to 9.0.x, and then cherry-picked it to 8.9.x. Thanks!