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

Enforce style guidelines with custom codespell dictionary #10221

Merged
merged 1 commit into from
Nov 12, 2024

Conversation

tetrapod00
Copy link
Contributor

@tetrapod00 tetrapod00 commented Nov 7, 2024

Adds a custom dictionary to codespell. Both the default codespell dictionary and our custom dictionary are checked, using multiple copies of the -D argument.

Currently, this dictionary only contains "anti-aliasing -> antialiasing". Later, we can add other style guidelines that we want to enforce, like perhaps "run-time -> runtime". #10218 tracks potential words to add.

Note that codespell is limited here. We can't check for phrases with spaces or for specific capitalizations. We also can't check for phrases which we don't want to enforce a style for 99%+ of the time, since each exception needs to be listed in an ignore file. So the number of entries in the custom dictionary will likely remain low.


Addresses #10218, but does not close.
This PR is a pure superset of #10220. It uses the same fix for codespell not checking some files. It also includes the one typo found by the other PR, so that the CI passes. However, I made two separate PRs because we probably want to merge the simple bug fix before this feature change.

@mhilbrunner
Copy link
Member

@tetrapod00 Reviewed and merged the other one first, so this now needs a rebase :) Looks good to me though.

@tetrapod00
Copy link
Contributor Author

Are you sure this one is a cherrypick all the way back? Unless I'm misunderstanding, won't the CI start complaining about some usages of "anti-aliasing" in old docs versions?

@mhilbrunner
Copy link
Member

Yeah, this would definitely also need to fix those occurences in older branches. Applying the labels is more of a suggestion to myself that this in theory does apply to old branches as well, and there is something to be said to have the "meta" stuff like Github Actions, CI, build system and ReadTheDocs stuff stay mostly in sync so we don't end up with 10 different setups eventually.

When actually doing cherry-picks, there is still some consideration that goes into what is actually cherry-picked. I'm not 100% decided on this one, and hey, now we got this comment trail to remind me to revisit that discussion :)

Thanks for the rebase.

@mhilbrunner mhilbrunner merged commit 905b1f4 into godotengine:master Nov 12, 2024
1 check passed
@mhilbrunner
Copy link
Member

In this goes 🎉 Thank you!

@tetrapod00 tetrapod00 deleted the codespell-custom branch November 12, 2024 06:57
@mhilbrunner
Copy link
Member

Cherrypicked to 4.3 in #10346.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants