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

ci: Configure crate-ci/typos and auto fix what I can #1551

Merged
merged 2 commits into from
Jul 17, 2024

Conversation

halkeye
Copy link
Member

@halkeye halkeye commented Jul 16, 2024

// ArgDatabaseTopicMesssageDownConversionEnable determines whether brokers should convert messages for consumers expecting older message formats
ArgDatabaseTopicMesssageDownConversionEnable = "message-down-conversion-enable"
// ArgDatabaseTopicMessageDownConversionEnable determines whether brokers should convert messages for consumers expecting older message formats
ArgDatabaseTopicMessageDownConversionEnable = "message-down-conversion-enable"
Copy link
Member

Choose a reason for hiding this comment

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

As this is a "code aware" spell checker, any chance you happen to know if there is a way to exclude existing exported vars/consts? That's not really an issue for doctl itself, but if we were to bring this into say godo, we'd want to avoid breaking changes.

Copy link
Member Author

Choose a reason for hiding this comment

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

so best i've found in the past was default.extend-identifiers in the config, but nothing automated
I believe typos also has a cli flag for listing all the identifiers it finds

that being said, fixing the spelling is an easy breaking change to handle for end users, its not like they have to figure out how to rewrite code because a function no longer exists.

Copy link
Member

@andrewsomething andrewsomething left a comment

Choose a reason for hiding this comment

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

Great stuff! I've been meaning to get to adding something like this. Thanks 🙏

Could you add a note somewhere in the CONTRIBUTING.md around how to run/set this up for local dev?

@halkeye
Copy link
Member Author

halkeye commented Jul 16, 2024

I can when I get a minute. Do you want a PR github action too?

@andrewsomething
Copy link
Member

Do you want a PR github action too?

If you're up for it. That would be great. We can add that latter too to no block this PR.

@halkeye
Copy link
Member Author

halkeye commented Jul 16, 2024

Do you want a PR github action too?

If you're up for it. That would be great. We can add that latter too to no block this PR.

#1552

decided to do it seperately

Copy link
Member

@andrewsomething andrewsomething left a comment

Choose a reason for hiding this comment

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

👍 Thanks again Gavin!

@andrewsomething andrewsomething merged commit 9128ab5 into digitalocean:main Jul 17, 2024
7 checks passed
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.

2 participants