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

Codespell #1939

Merged
merged 6 commits into from
Jul 1, 2021
Merged

Codespell #1939

merged 6 commits into from
Jul 1, 2021

Conversation

dopplershift
Copy link
Member

@dopplershift dopplershift commented Jun 28, 2021

Description Of Changes

  • Add codespell to our set of linting tools
  • Correct errors
  • Fix all spellings (as possible--thta was in a previous function signature) of "thta" with "theta"

Checklist

@dopplershift dopplershift added Area: Infrastructure Pertains to project infrastructure (e.g. CI, linting) Type: Feature New functionality labels Jun 28, 2021
@dopplershift
Copy link
Member Author

I have a commit for "pres" -> "press", but I'm not sure I actually want to make that change. "thta" -> "theta" corrects a weird and unnecessary move away from a word. "pres" vs. "press" is just a different length of abbreviation.

@kgoebber
Copy link
Collaborator

I would say stay with "pres" unless we wanted to go to the full name of pressure. I wouldn't intuitively think to use "press".

@dcamron
Copy link
Member

dcamron commented Jun 29, 2021

Would just going to pressure everywhere make anything specific less readable or too verbose? If not I'd be fine with that.

@dopplershift
Copy link
Member Author

So changing "pres" to "pressure" only resulted in one new line wrap, so I went ahead and did that. More importantly, I figured out how to exclude a few false alarms (involves adding the entire content of the line to a file), but this allows us to have "pres" and "thta" as checked words, and only exclude a couple of known ok occurrences--so I'm much more interested in having them changed in that case.

Copy link
Member

@dcamron dcamron left a comment

Choose a reason for hiding this comment

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

Catches things pretty smoothly! Fan of this, and looks good to go. Just a comment line copied in the workflow.

.github/workflows/linting.yml Outdated Show resolved Hide resolved
Also work around a couple (reasonable) false alarms.
This leaves alone the API change notice, so we can't remove the ignore
for "thta".
This allows us to exclude specific entire lines (by full line content)
from flagging errors. This allows us to eliminate the few remaining
occurrences of "pres" and "thta" that should remain, but flag any new
occurrences.
@dcamron dcamron merged commit 3dc05c8 into Unidata:main Jul 1, 2021
@github-actions github-actions bot added this to the 1.1.0 milestone Jul 1, 2021
@dopplershift dopplershift deleted the codespell branch July 1, 2021 19:15
@dopplershift
Copy link
Member Author

I'm always a fan of adding tools that when I run them, they catch a bunch of stuff I would have liked to notice during code review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Infrastructure Pertains to project infrastructure (e.g. CI, linting) Type: Feature New functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add codespell to suite of linters
3 participants