- Issue created by @jonathan1055
- š¬š§United Kingdom jonathan1055
Ready for testing and feedback. But I undertand if you do not think this is our job to maintain. However, it is gitlab_templates decision to keep up with the core version, so we are causing contrib projects to fail when we bump to the next version. This won't be the last time that words are dropped.
- š¬š§United Kingdom jonathan1055
The name "extra-dictionary.txt" is not very imaginative. If anyone has a better idea, let me know. It is not just for words dropped from core, so I did not want to include 'core' in the name.
- šŖšøSpain fjgarlin
Iām thinking that while we have a handful of them only we might want to keep them in the array that will also populate the artifacts, instead of as a separate file.
That way if maintainers see the cspell config file with those words they might choose to keep them or not.
If or when we have more words we might consider a separate file but if we want to do a quick-fix Iād do it in the most simple way. Thoughts?
- š¬š§United Kingdom jonathan1055
OK. I will revert the initial work, and just add the words via the array in .cspell.json.
But first, I wanted to record the tests using this extra dictionary, just to see how it worked.
Here's a test with new words, using existing gitlab templates 1.6.11 running core 11.1
https://git.drupalcode.org/issue/scheduler-3445052/-/pipelines/373557Using MR310 with new dictionary
https://git.drupalcode.org/issue/scheduler-3445052/-/pipelines/373584
The job passes and the cspell.json artifact has the new dictionary - šŖšøSpain fjgarlin
Thanks for running the tests. With the external file approach, we'd need to include it as well in the artifacts, and for example, in the second run, the cspell.json file has
"path": "./extra-dictionary.txt",
but the artifact is not in there.I think that if we see the number of words growing to more than 15-20, we can do the extra file/artifact in a follow-up, but for now it might just be easier to add the files to the array as we had "ddev" and "lando". The only reason that I'm pushing for this simpler approach is that the solution would be lower risk and we can merge it and create a new release before tomorrow.
- š¬š§United Kingdom jonathan1055
I have pushed the changes, and re-tested on the same branch:
https://git.drupalcode.org/issue/scheduler-3445052/-/pipelines/373633
The cspell.json artifact has the new words (and not the new dictionary)Ready for review.
- šŖšøSpain fjgarlin
Looks good to me. Thanks so much!
As per #7, I'm happy if a follow-up issue is created to move those array of words into a separate file, where we can improve this and test it properly, but without the pressure of getting it out quick that this issue has.
-
fjgarlin ā
committed ab7fe3eb on main authored by
jonathan1055 ā
Issue #3494834: Add common words not in core dictionaries
-
fjgarlin ā
committed ab7fe3eb on main authored by
jonathan1055 ā
- šŖšøSpain fjgarlin
Merged. I'll tag a new release and make it default-ref.
-
fjgarlin ā
committed 4df6d2cd on main
#3494834: Changelog.
-
fjgarlin ā
committed 4df6d2cd on main