Postponed on #3258435: Remove deprecated feed.reader.* and feed.writer.* services from the container
Problem/Motivation
Most of these words are used once in a single file, some are in two files.
archiver
atomentry
atomfeed
atomrendererfeed
chainedfast
checkboxradio
configentity
configurator
contententry
contentrendererentry
controlgroup
derivedfrom
dublincoreentry
dublincorefeed
dublincorerendererentry
dublincorerendererfeed
itunesentry
itunesfeed
itunesrendererentry
itunesrendererfeed
kinberg
linkback
lrdd
mattfarina
phpserialize
pingback
podcastentry
podcastfeed
preconnect
refback
robloach
sayre
slashrendererentry
slatkin
threadentry
threadingrendererentry
timegate
timemap
trackback
webmention
wellformedwebentry
wellformedwebrendererentry
Remaining tasks
Review
| Comment | File | Size | Author |
|---|---|---|---|
| #23 | interdiff_21_23.txt | 457 bytes | beatrizrodrigues |
| #23 | 3210125-23.patch | 8.36 KB | beatrizrodrigues |
| #21 | 3210125-21.patch | 8.4 KB | beatrizrodrigues |
| #17 | 3210125-17.patch | 8.43 KB | quietone |
| #17 | interdiff-15-17.txt | 2.51 KB | quietone |
Issue fork drupal-3210125
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:
Comments
Comment #2
quietone commentedNot testing yet.
Is this the way to go for these files?
Comment #3
quietone commentedComment #4
longwaveThis looks nicely self contained, I hope this is an easy win to get in.
Let's put these in alphabetical order.
Comment #8
rakesh.gectcrComment #10
rakesh.gectcrHi all, Please forgive, after a long time, back into issue queues. First time working with drupal core issue forking workflow. So please advise. Sorry for the mess up on the branch.
However, I have addressed the changes suggested by @longwave.
Please review.
Thanks,
Kind regards,
Rakesh James
Comment #11
rakesh.gectcrComment #12
quietone commentedHI rakesh.gectcr, thanks for the updates.
Since longwave likes this method, I have added two words for two more files, core.libraries.yml and and core.services.yml. I've also added a couple of words from those words that appear in two files. There are words in these files that will require changes in 3 or more other files and that just seems too much. It is always a guess but this seems like a nice set of words.
Not much a fan of gitlab workflow so this is a patch.
Comment #13
quietone commentedComment #15
quietone commentedRerolling for 9.3.x. Removed the changes to popperjs because I was getting a failure is a .js file and I don't know how to work with js files.
Comment #16
longwaveI think there should be no space betweeen cspell: and ignore.
These lines could be wrapped more tightly.
Not convinced we should remove this, "archiver" is a valid word.
Where did this come from?
Comment #17
quietone commented16.
1 and 2. fixed.
3. archiver restored to dictionary
4. I have no idea. removed.
Comment #19
spokjeDoesn't apply on
9.4.x-devany moreComment #20
beatrizrodriguesI'll do the reroll
Comment #21
beatrizrodriguesHere's the reroll.
Comment #22
spokjeThanks for the reroll.
Looks like all the additions to
/core/misc/cspell/dictionary.txtin #21 are unneeded and probably removed in an already committed issue between #17 and now.So basically, I think we can get rid off:
Comment #23
beatrizrodriguesthanks @Spokje Here's a new patch that addresses your comment. And a interdiff too. Hope it fits.
Comment #24
spokjeThanks, RTBC for me now :)
Comment #25
catchI'm not sure about this:
1. The .yml files get read by humans a lot more often than dictionary.txt, this adds a lot of noise to them.
2. #2979588: Deprecate Laminas\Feed reader and writer services will remove a lot of the words in core.services.yml anyway.
3. Why not have drupalci in the dictionary?
Comment #26
longwave#25.1 Should we consider excluding e.g. core.services.yml from being spellchecked instead? Note that doing this has picked up a minor typo in a test (Configentity vs ConfigEntity) so I think leaving them all as dictionary words is not necessarily the right thing to do either.
Agree that #25.2 will eventually remove a lot of these exceptions anyway.
Comment #27
quietone commentedGood points in #25 and #26.
Ignoring core.services.yml makes a lot of sense to me, it seems likely to always have words that will fail spelling.
Regarding Configentity, while it was found because of looking for words used once in yml files, it could easily be discovered by other means, say, doing words beginning with 'c'. I was searching for words used once to get quick wins to reduce the size of the dictionary.
How about just postponing this until #2979588: Deprecate Laminas\Feed reader and writer services.
Comment #28
longwave#2979588: Deprecate Laminas\Feed reader and writer services is in but I think we need to wait for #3258435: Remove deprecated feed.reader.* and feed.writer.* services from the container instead.
Comment #29
longwaveRe: core.services.yml: the only English text is a handful of comments (less than 3% of the file) and deprecation messages - but any deprecation should also have an accompanying test which will still be spellchecked. Therefore if we can accept that the small number of comments will not be spellchecked I think it is worth it to avoid many false words that need ignoring one way or another.
Comment #30
quietone commentedUpdate the IS to show the issue this is postponed on.
Comment #33
quietone commentedAdding tag for the cspell spelling error issues.