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

[Breaking change] remove "Key" suffix from ApiKey variants #97

Merged

Conversation

rukai
Copy link
Collaborator

@rukai rukai commented Nov 21, 2024

Similar change to #61

The "Key" suffix is redundant since the variants are usually written out as ApiKey::ProduceKey.

Additionally its common to log a key type by doing something like log!("failed to parse message type {api_key:?}"), in this case including the Key is confusing because it seems like Key is part of the message name.

@rukai rukai force-pushed the remove-"Key"-suffix-from-ApiKey-variants branch from ce4b2c5 to 756a02b Compare November 21, 2024 01:02
@rukai rukai force-pushed the remove-"Key"-suffix-from-ApiKey-variants branch from 756a02b to ae5a3cb Compare November 21, 2024 01:04
@rukai
Copy link
Collaborator Author

rukai commented Dec 1, 2024

@tychedelia This PR is very similar to the PR #61 which was merged without issue, so I'll consider this non-controversial and merge it tomorrow.

Copy link
Owner

@tychedelia tychedelia left a comment

Choose a reason for hiding this comment

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

Yup, lgtm!

@tychedelia tychedelia added this to the 0.14.0 milestone Dec 2, 2024
@tychedelia tychedelia added the enhancement New feature or request label Dec 2, 2024
@tychedelia tychedelia merged commit b238d0b into tychedelia:main Dec 2, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants