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

Add a list of accepted "misspelled" words #1863

Merged
merged 4 commits into from
Dec 20, 2022
Merged

Conversation

IndrajeetPatil
Copy link
Collaborator

No description provided.

@MichaelChirico
Copy link
Collaborator

at a glance this seems like more of a maintenance burden than it's worth.

Why have this in CI, as opposed to part of the release procedure?

@IndrajeetPatil IndrajeetPatil marked this pull request as ready for review December 20, 2022 05:35
@codecov-commenter

This comment was marked as off-topic.

@IndrajeetPatil IndrajeetPatil changed the title Add spell check test Add a list of accepted "misspelled" words Dec 20, 2022
@IndrajeetPatil
Copy link
Collaborator Author

I have removed the spelling test to not gain an additional soft dependency.

The idea here is to keep a list of words which we know to be acceptable words, so that we don't need to go through the list every single time we make a release.

Currently, on main, devtools::spell_check() generates a huge list we need to go through to see which are truly misspelled words.

If we add a list like the one in this PR, only the newly introduced, misspelled words will be flagged. This reduces maintainance burden for us.

@MichaelChirico
Copy link
Collaborator

we don't need to go through the list every single time we make a release.

right but there will presumably be some churn, every time new documentation/vignettes are added there's likely to be some new words to add.

happy to try it out for a few releases to see just how big that delta is each time though

io
jwz
knitr
labelled
Copy link
Collaborator

Choose a reason for hiding this comment

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

labelled is British spelling, we should be consistent

inst/WORDLIST Outdated
eg
envvar
eval
favour
Copy link
Collaborator

Choose a reason for hiding this comment

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

same, we should be consistent

inst/WORDLIST Outdated
alllowercase
backports
backticked
behaviour
Copy link
Collaborator

Choose a reason for hiding this comment

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

another

inst/WORDLIST Outdated
debian
dplyr
dragosmg
eg
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should use e.g. consistently

inst/WORDLIST Outdated
rlang
roxygen
sandboxing
spilt
Copy link
Collaborator

Choose a reason for hiding this comment

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

spilt is British

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is going to be a tough one for someone like me who comes from the Commonwealth. 😅

Copy link
Collaborator

Choose a reason for hiding this comment

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

Just be happy we don't need to decide about metric units 😂

TBH I'm comfortable switching to en-UK too, but we should be consistent (do we have any function names assuming stateside spelling?).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thankfully, no, we don't have any function names that assume US spellings.

inst/WORDLIST Outdated
linter
linters
lintr's
localisation
Copy link
Collaborator

Choose a reason for hiding this comment

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

one more 🇬🇧

inst/WORDLIST Outdated
nd
nodeset
nolint
parameterised
Copy link
Collaborator

Choose a reason for hiding this comment

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

one more 🇬🇧

@MichaelChirico MichaelChirico merged commit 7dfe5d6 into main Dec 20, 2022
@MichaelChirico MichaelChirico deleted the add_spell_check branch December 20, 2022 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants