Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OSOE-84: New words to allow for Node.js Extensions #186

Merged
merged 30 commits into from
Jan 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
59a3086
Merge remote-tracking branch 'origin/dev' into issue/OSOE-84-final
Dec 8, 2022
e06a304
Revert reversion of our precious code
Dec 8, 2022
8b4088a
Merge remote-tracking branch 'origin/dev' into issue/OSOE-84-final
Dec 9, 2022
ac5cbe2
Merge remote-tracking branch 'origin/issue/OSOE-490' into issue/OSOE-…
Dec 9, 2022
68247f5
Shorten the artifact name
Dec 9, 2022
22b25b5
Merge remote-tracking branch 'origin/dev' into issue/OSOE-84-final
Dec 12, 2022
b8903cc
Add iis and localhost to allow.txt
Dec 12, 2022
6a26927
Merge remote-tracking branch 'origin/dev' into issue/OSOE-84-final
Dec 14, 2022
bdd2b59
Merge remote-tracking branch 'origin/dev' into issue/OSOE-84-final
Dec 15, 2022
db8b854
Merge remote-tracking branch 'origin/dev' into issue/OSOE-84-final
Dec 16, 2022
348ca66
Merge branch 'dev' into issue/OSOE-84-final
Dec 28, 2022
7b163e1
Allowing more words for NE
Dec 28, 2022
023eac7
Merge remote-tracking branch 'origin/dev' into issue/OSOE-84-final
Dec 28, 2022
d767364
Merge remote-tracking branch 'origin/dev' into issue/OSOE-84-final
Dec 28, 2022
222d2e4
Merge remote-tracking branch 'origin/dev' into issue/OSOE-84-final
Dec 29, 2022
2a17f89
Merge remote-tracking branch 'origin/dev' into issue/OSOE-84-final
Dec 30, 2022
f93bec9
Merge remote-tracking branch 'origin/dev' into issue/OSOE-84-final
Jan 2, 2023
54b7194
Merge remote-tracking branch 'origin/dev' into issue/OSOE-84-final
Jan 7, 2023
6037f99
Merge remote-tracking branch 'origin/dev' into issue/OSOE-84-final
Jan 9, 2023
4045e02
Merge remote-tracking branch 'origin/dev' into issue/OSOE-84-final
Jan 13, 2023
1f4ad7f
Add symlinks and symlinked to dictionary
Jan 13, 2023
cedc130
Merge branch 'dev' into issue/OSOE-84-final
Jan 13, 2023
9eda721
Merge remote-tracking branch 'origin/dev' into issue/OSOE-84-final
Jan 16, 2023
69b83f2
Merge branch 'dev' into issue/OSOE-84-final
Jan 18, 2023
2ff8ddb
Temporarily removing new entries to see where they are used
Jan 18, 2023
9495209
Re-add necessary new entries
Jan 18, 2023
32cc827
Revert "Shorten the artifact name"
Jan 18, 2023
8aa042e
Merge branch 'dev' into issue/OSOE-84-final
Jan 18, 2023
87277a1
revert branch selector
sarahelsaig Jan 24, 2023
1e2d065
revert branch selector
sarahelsaig Jan 24, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/actions/spelling/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,12 @@ inputs:
cspell:rust/rust.txt
cspell:scala/scala.txt
cspell:software-terms/src/software-terms.txt
cspell:node/node.txt
cspell:typescript/typescript.txt
config:
description: Spelling configuration directory
required: false
default: .github/actions/check-spelling
default: .github/actions/spelling
spell-check-this:
description: Repository with default configuration to use, the default from Check Spelling is ''.
required: false
Expand Down
12 changes: 12 additions & 0 deletions .github/actions/spelling/allow.txt
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ apiclienttenant
apikey
apps
appsettings
argv
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of these look generic enough for the common configuration, but some very rare. For the latter, rather use #spell-check-ignore-line

Copy link
Contributor Author

@0liver 0liver Jan 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, most of those rare words are simply specific to NE. Are we planning to support project-level dictionaries anytime soon (@BenedekFarkas)? Because then I'd rather move them into one of those than commenting them everywhere.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, good point, the spelling configuration in most repositories (but definitely in OSOCE) will be completely overhauled in Lombiq/Open-Source-Orchard-Core-Extensions#346.
So, you can apply the current practices to make it easier on rolling this out without having to worry about upcoming changes.

See my comment about using per-line ignores here: #134 (comment)
If you can expect that a specific word will appear in more places in the future, then putting it in a dictionary is OK. Based on applying this concept to OrchardCMS/OrchardCore.Commerce#250 and some other repos, I'd say that maximum 1 or 2 occurrences of a rare word (if you don't expect more) is OK to ignore.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the record - all errors by the spell checker when no new entries are added:

Autoprefix is not a recognized word. (unrecognized-spelling)
EBUSY is not a recognized word. (unrecognized-spelling)
EOL is not a recognized word. (unrecognized-spelling)
EOL is not a recognized word. (unrecognized-spelling)
EOL is not a recognized word. (unrecognized-spelling)
Lintrc is not a recognized word. (unrecognized-spelling)
Lintrc is not a recognized word. (unrecognized-spelling)
Promisified is not a recognized word. (unrecognized-spelling)
Resx is not a recognized word. (unrecognized-spelling)
argv is not a recognized word. (unrecognized-spelling)
argv is not a recognized word. (unrecognized-spelling)
argv is not a recognized word. (unrecognized-spelling)
argv is not a recognized word. (unrecognized-spelling)
argv is not a recognized word. (unrecognized-spelling)
binstubs is not a recognized word. (unrecognized-spelling)
chdir is not a recognized word. (unrecognized-spelling)
chdir is not a recognized word. (unrecognized-spelling)
chdir is not a recognized word. (unrecognized-spelling)
chdir is not a recognized word. (unrecognized-spelling)
constitue is not a recognized word. (unrecognized-spelling)
copyfiles is not a recognized word. (unrecognized-spelling)
copyfiles is not a recognized word. (unrecognized-spelling)
copyfiles is not a recognized word. (unrecognized-spelling)
copyfiles is not a recognized word. (unrecognized-spelling)
copyfiles is not a recognized word. (unrecognized-spelling)
errored is not a recognized word. (unrecognized-spelling)
lintrc is not a recognized word. (unrecognized-spelling)
loglevel is not a recognized word. (unrecognized-spelling)
npmrc is not a recognized word. (check-file-path)
npmrc is not a recognized word. (check-file-path)
npmrc is not a recognized word. (unrecognized-spelling)
promisify is not a recognized word. (unrecognized-spelling)
promisify is not a recognized word. (unrecognized-spelling)
symlink is not a recognized word. (unrecognized-spelling)
symlink is not a recognized word. (unrecognized-spelling)
symlinked is not a recognized word. (unrecognized-spelling)
symlinked is not a recognized word. (unrecognized-spelling)
symlinks is not a recognized word. (unrecognized-spelling)
textlint is not a recognized word. (unrecognized-spelling)
textlint is not a recognized word. (unrecognized-spelling)
textlint is not a recognized word. (unrecognized-spelling)
thenable is not a recognized word. (unrecognized-spelling)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's not too bad! Just add them here to allow.txt for now (or ignore in-code whatever you see fit for that) and I'll take care of the rest during the refactoring.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did what Zoltán suggested - ignored the rare ones, re-added the common ones.

ASPNETCORE
asynchronicity
atata
Expand All @@ -19,6 +20,7 @@ beforeunload
bootstraptab
browsersync
Cdp
chdir
chromedriver
cloudsmith
cmdlet
Expand All @@ -37,6 +39,7 @@ contentitem
contentitemid
contenttype
contextmenu
copyfiles
corepack
createstate
crontab
Expand All @@ -62,6 +65,7 @@ endcapture
endgroup
endzone
Enums
EOL
EUS
eventname
executesql
Expand Down Expand Up @@ -111,7 +115,9 @@ JToken
JValue
langword
linq
lintrc
listheader
loglevel
logtest
lombiq
lucene
Expand All @@ -129,12 +135,14 @@ mvc
Namespaced
NCrontab
Newtonsoft
npmrc
paramref
permalink
plugin
plugins
Portainer
prebuilt
promisify
readme
Reappend
refactoring
Expand Down Expand Up @@ -168,12 +176,16 @@ subfolders
submenu
Subresource
superproject
symlink
symlinks
symlinked
taghelper
taxonomyexpensetags
taxonomyfield
taxonomyincometags
teamcity
testuser
textlint
timespan
Timestamped
Timezones
Expand Down
2 changes: 2 additions & 0 deletions .github/actions/spelling/excludes.txt
Original file line number Diff line number Diff line change
Expand Up @@ -31,5 +31,7 @@
^LICENSE\.txt$
ignore$
pnpm-lock\.yaml$
# For Lombiq.DataTables
UnmanagedNodeModules/
UploadingTestFileDOCX\.docx$
UploadingTestFileXLSX\.xlsx$