Problem/Motivation

Discovered at #2972224: Add .cspell.json to automate spellchecking in Drupal core, and pointed by @xjm in https://www.drupal.org/project/drupal/issues/3122088#comment-13628724

+++ b/core/.cspell.json
@@ -0,0 +1,1288 @@
+      "cotrol",

Proposed resolution

@xjm: Probably "control".

As title and see the change record https://www.drupal.org/node/3122084 for how to work with cspell.

Remaining tasks

Pick out all applicable words from #2972224: Add .cspell.json to automate spellchecking in Drupal core and fix them.

User interface changes

API changes

Data model changes

Release notes snippet

Comments

jungle created an issue. See original summary.

sja112’s picture

Assigned: Unassigned » sja112
sja112’s picture

Assigned: sja112 » Unassigned
Status: Active » Needs review
StatusFileSize
new1.36 KB
kunalgautam’s picture

StatusFileSize
new104.14 KB

#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

ketikagrover’s picture

Status: Needs review » Reviewed & tested by the community
~/Documents/9-drupal on 9.1.x [!?$]
$ grep -r "cotrol" *
~/Documents/9-drupal on 9.1.x [!?$]
$ 

Shows empty. Moving this to RTBC.
Thanks!

xjm’s picture

Status: Reviewed & tested by the community » Needs work

Now 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!

jungle’s picture

Status: Needs work » Needs review
StatusFileSize
new1.66 KB
new1.65 KB

Removed it from the dictionary, and fixed "over 80 chars".

jungle’s picture

Status: Needs review » Reviewed & tested by the community

As the changes in #7 is small/tiny, setting back to RTBC.

xjm’s picture

Title: Fix "control" relevant typos in core » Fix the typos "cotrol" and make the one-line summaries containing it conform to standards)
Status: Reviewed & tested by the community » Needs work

Ah 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.):

Makes assertions about a password reset triggering user flood control.

That should hopefully fit.

Thanks!

mohrerao’s picture

@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

jungle’s picture

Nice 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.

jungle’s picture

As suggested, remove "Helper function to..." would be enough, -- less than 80 chars, and all in one line.

mohrerao’s picture

Status: Needs work » Needs review
StatusFileSize
new2.45 KB
new2.1 KB

Adding a patch fixing one-line summary.

jungle’s picture

Status: Needs review » Reviewed & tested by the community

Thanks, @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.

jungle’s picture

StatusFileSize
new0 bytes

Well, upload a patch for 9.0.x

mohrerao’s picture

StatusFileSize
new5.46 KB

Adding a patch for 9.0.x

jungle’s picture

StatusFileSize
new2.17 KB

Funny, both #15 and #16 are wrong.

  • xjm committed 69fc22c on 9.1.x
    Issue #3138796 by jungle, mohrerao, sja112, kkalashnikov, xjm,...

  • xjm committed 46921bc on 9.0.x
    Issue #3138796 by jungle, mohrerao, sja112, kkalashnikov, xjm,...

  • xjm committed 74912f0 on 8.9.x
    Issue #3138796 by jungle, mohrerao, sja112, kkalashnikov, xjm,...
xjm’s picture

Status: Reviewed & tested by the community » Fixed

Yeah, 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!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.