Problem/Motivation

This is a follow-up to #3521924: Convert Coding Standards to GitLab pages. We need to run cspell to check spelling in all the documentation files.

These are the remaining misspelled words.
rdbms - Valid abbrevation,

Person's name

  • celko
  • gazit
  • idan

likely to be removed in #3324368: Update CSS coding standards to include PostCSS and Drupal 10

  • buttongroup
  • csscomb
  • csscombx
  • gulpfile
  • invisibles
  • microformats
  • navbars
  • oocss
  • transpiles
  • vimrc

Issue fork coding_standards-3527481

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

jonathan1055 created an issue. See original summary.

jonathan1055’s picture

jonathan1055 commented

[removed]

jonathan1055 opened merge request !2

jonathan1055’s picture

jonathan1055 commented
Status: Active » Needs review

I have pushed an initial commit to check what happens with no customized skip or opt-in settings. The four validation/linting jobs that are run are:

  • composer-lint - this checks the overall syntax of files, there is no harm in running it. But it may not be needed and just wasting resource, so could skip it.
  • cspell - we want this
  • eslint - this checks the formatting of .yml files, so we should keep this
  • phpcs - I don't know what this is checking (maybe the the .md files?). It passes, so is OK. But we may decide to skip it

The two validation/linting jobs which do not get run are phpstan (we have no .php files) and stylelint (we have no .css files)

MR2 is ready for review and feedback.

jonathan1055’s picture

jonathan1055 commented

Initial cspell results

CSpell: Files checked: 37, Issues found: 105 in 17 files.
The number of distinct unrecognised/misspelled words is 51

The shortest wordlength defaults to 4, so for some made-up ids we can shorten it from 4 to 3 letters. I fixed the 'lorem ipsum' phrases. The words are in a standard dictionary, but the pages were not using the generally agreed spellings, so I fixed that. Plus some actually misspelled words, and UK -> US English variants. Re-run and we have the reduced counts:

CSpell: Files checked: 37, Issues found: 77 in 15 files.
The number of distinct unrecognised/misspelled words is 34

There are various ways to proceed here, we can create a project words file to add the unrocognised words, but its probably better to make the docs use real words if possible. Some words may need to be added to a custom dictionary, but not all of them.

jonathan1055’s picture

jonathan1055 commented

Ignoring the section of the file that has SQL keywords made a big improvement

CSpell: Files checked: 37, Issues found: 63 in 13 files.
The number of distinct unrecognised/misspelled words is 25

I have also added a project dictionary (using the default .cspell-project-words.txt filename), and we get down to:

CSpell: Files checked: 37, Issues found: 38 in 8 files.
The number of distinct unrecognised/misspelled words is 20

I will stop here, and let you review and give feedback, before doing any more additions or corrections.

dww’s picture

dww
we/he/they
commented

So far, changes mostly look good. Opened 1 MR thread, though.

Also, since cspell is the primary thing we want CI to enforce, and since there are no other “tests” we might run, regardless of spelling errors, shall we set allow_failure: false for that job?

quietone changed the visibility of the branch 3527481-run-cspell-and-other-jobs to hidden.

quietone changed the visibility of the branch 3527481-run-cspell-and-other-jobs to active.

quietone’s picture

quietone commented

I worked on removing words for the local list and the errors reported.

That brings the errors down to 14, which I'd be find moving to the local project words. Although, maybe gulpfile and navbars can be changed?

That leaves the sql words and changing to failing on spelling errors.

quietone’s picture

quietone commented

It may be the lack of navigation that is making the GitLab pages. Although I made a separate issue for that shall we add that here. I have the nav locally and it works locally. So, let me know if it can be added here. Thanks.

jonathan1055’s picture

jonathan1055 commented

I have added the change to make the job fail red, not amber warning. Nice work on editing to remove more unknown words. The only question I have is that you have changed all mymodule into my_module to remove 'mymodule' from the project dictionary. But I would suggest that "mymodule" is quite a common word used in program documentaion and examples, so I thought it would be better to allow that word. If we don't then future updates are likely to fail on spelling and we'll end up with varying versions of myModule, my_module, and other examples. It's fine if you want to stick to my_module, I'm just raising the discussion :-)

I have the nav locally and it works locally. So, let me know if it can be added here.

I suggest we focus here on the spelling, and do the navigation on #3527422: Add navigation where I have already made some comments about it.

quietone’s picture

quietone commented

It may be common, it is also a misspelling. It was removed from Drupal core dictionary.txt in Feb 2024, #3417675: Correct mymodule, mydriver and anothermodule. So, we should re-introduce a misspelling in any of the examples.

quietone’s picture

quietone commented
Issue summary: View changes

@jonathan1055, thanks for changing this to fail.

I updated the IS with the remaining misspelled words and where they are used. I then added them to the local dictionary. We know have a green run.

jonathan1055’s picture

jonathan1055 commented
Status: Needs review » Reviewed & tested by the community

[mymodule] may be common, it is also a misspelling.

Ah. OK thanks for that background. Yes of course we don't want to add it back.

Now that you've added those custom words to the project dictionary this is RTBC. But also so is MR4. So which ever you merge first, I can rebase the other and resolve the conflict.

  • quietone committed 9c6c0eab on main 
    Issue #3527481 by jonathan1055, quietone, dww: Run cspell and other...
quietone’s picture

quietone commented
Status: Reviewed & tested by the community » Fixed

The point @dww made was answered but we haven't heard back. Since that can changed in another issue I have committed this in order to get the these jobs running.

Thanks folks!

quietone closed merge request !2

Status: Fixed » Closed (fixed)

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