Closed (fixed)
Project:
Drupal core
Version:
9.2.x-dev
Component:
other
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
6 May 2021 at 11:39 UTC
Updated:
26 May 2021 at 10:49 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
spokjeI've ran
cd core && yarn install && yarn run spellcheck -c .cspell.json "/path/to/drupal/**/*"on9.3.x.After deleting a lot of
/vendorwarnings (I know there must be a command to exclude directories, too lazy to google more than once ;), the results aren't as bad as expected":Still needs fixing though, opening a MR.
Comment #4
spokjeAlthough it looks like adding a
/* cspell:disable */tocore/modules/color/preview.htmlpasses all color module related tests (See: https://git.drupalcode.org/project/drupal/-/blob/475bc9f6339d1a7194368b455d7ee5108815acdb/core/modules/color/preview.html and https://www.drupal.org/pift-ci-job/2053211), I'm highly doubtful this is what we want.Firing a new test with only changes in
core/modules/migrate/src/Plugin/migrate/source/SourcePluginBase.phpand let reviewers/committers decide what they wanna do.Comment #6
spokjeSince
9.2.xhas the same unknown word list as9.3.x, the same patch could be applied over there.No clue about the status of
9.1.xwhich is freezed according to @catch #3212521-12: [backport] cspell dislikes identifer in core/modules/views/src/Plugin/views/filter/FilterPluginBase.php and will fail any patch touching that file, so I'll just leave that alone.Which MR to use is at reviewers/committers discression:
MR641: 2 changes, not too sure about
core/modules/color/preview.htmlMR642: 1 change
Comment #7
alexpottIt'd be cool if core/scripts/dev/commit-code-check.sh could detect a change in the dictionary and re-run the tests. We can test if the dictionary is in $FILES - also we should only do this on DrupalCI - there's a flag for that too. Also we could extend this to:
Comment #8
spokjeCoolness affirmed, follow-up in #3212579: Spell check all files if dictionary.txt changes
Comment #9
longwaveWe already have this line in .cspell.json:
Maybe we need to change that to
Comment #13
spokjeNice catch @longwave!
That indeed does the trick, created new MR with that addition and sunk the two existing MRs to prevent confusion.
Comment #14
alexpottThe command to correctly run spellcheck on core is
cd core && yarn install && yarn run spellcheck:coreComment #15
alexpottComment #16
spokjeComment #17
quietone commentedI ran spellcheck on the code base before applying this patch and there was one error, iteratable. I applied the diff and ran commit-code-check and no errors reported.
This is good to go.
Comment #18
alexpottThere's another side of this we should re-run the command to make our custom dictionary because maybe more words have been added upstream... so we need to run
yarn run spellcheck:make-drupal-dict- note it is important that this is done on a clean checkout with no changes to the root composer.jsonComment #19
quietone commented@alexpott, thanks.
New patch created after running
yarn run spellcheck:make-drupal-dict. Among the changes to the dictionary it adds 'iteratable' so this does not include the fix for that word in SourcePluginBase. It seems best to handle that in #3122088: [Meta] Remove spelling errors from dictionary.txt and fix them. Hope that is right.Comment #20
longwave#3210633: Update JavaScript dependencies for Drupal 9.2 is also trying to fix the dictionaries, maybe we should just get this in here and then we don't need to worry about it over there.
I get a clean run of
yarn spellcheck:corewith #19 applied so this is RTBC.Comment #24
catchCommitted/pushed to 9.3.x and cherry-picked to 9.2.x, thanks!