-
Notifications
You must be signed in to change notification settings - Fork 605
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
refactor(proto): add LATEST
const value for proto message version enums
#20359
Conversation
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
This stack of pull requests is managed by Graphite. Learn more about stacking. |
PbAggNodeVersion::LATEST
LATEST
const value for proto message version enums
// in case DEFAULT_KEY_COLUMN_NAME changes | ||
COLUMN_DESC_VERSION_PR_13707 = 1; | ||
|
||
// for test only | ||
COLUMN_DESC_VERSION_MAX = 2147483647; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing MAX
doesn't matter because this should only be used in unit tests before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we consider deriving it? (like https://docs.rs/strum/latest/strum/derive.EnumIter.html)
Signed-off-by: Richard Chien <[email protected]>
Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
By adding a
LATEST
const value to version enums likerisingwave_pb::plan_common::ColumnDescVersion
, we can directly use this value when constructing new proto messages, avoiding updating all occurrence everytime we add a new version.Checklist
Documentation
Release note