Skip to content

ABCI++: Update new protos to use enum instead of bool#8158

Merged
mergify[bot] merged 11 commits intomasterfrom
wb/issue-8039
Mar 21, 2022
Merged

ABCI++: Update new protos to use enum instead of bool#8158
mergify[bot] merged 11 commits intomasterfrom
wb/issue-8039

Conversation

@williambanfield
Copy link
Contributor

closes: #8039

This pull request updates the new ABCI++ protos to use enums in place of bools. enums may be preferred over bool because an enum can be udpated to include new statuses in the future, whereas a bool cannot and is fixed as just true or false over the whole lifecycle of the API.

Copy link

@creachadair creachadair left a comment

Choose a reason for hiding this comment

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

LGTM, modulo the lint scribble.

Copy link
Contributor

@sergio-mena sergio-mena left a comment

Choose a reason for hiding this comment

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

Thanks a lot for taking care of this!

@williambanfield williambanfield added the S:automerge Automatically merge PR when requirements pass label Mar 21, 2022
@williambanfield
Copy link
Contributor Author

@thanethomson heads up, this change now re-orders the .proto.intermediate file to match the .proto file to make inspecting the diff between the two a bit easier.

@mergify mergify bot merged commit cc838a5 into master Mar 21, 2022
@mergify mergify bot deleted the wb/issue-8039 branch March 21, 2022 16:57
@thanethomson
Copy link
Contributor

@thanethomson heads up, this change now re-orders the .proto.intermediate file to match the .proto file to make inspecting the diff between the two a bit easier.

Great stuff! 👍

@sergio-mena sergio-mena mentioned this pull request Jul 22, 2022
35 tasks
williambanfield added a commit that referenced this pull request Sep 9, 2022
closes: #8039

This pull request updates the new ABCI++ protos to use `enum`s in place of `bool`s. `enums` may be preferred over `bool` because an `enum` can be udpated to include new statuses in the future, whereas a `bool` cannot and is fixed as just `true` or `false` over the whole lifecycle of the API.
@williambanfield williambanfield restored the wb/issue-8039 branch September 9, 2022 16:11
faddat pushed a commit to notional-labs/tendermint that referenced this pull request Apr 3, 2023
* [cherry-picked] ABCI++: Update new protos to use enum instead of bool (tendermint#8158)

This pull request updates the new ABCI++ protos to use `enum`s in place of `bool`s. `enums` may be preferred over `bool` because an `enum` can be udpated to include new statuses in the future, whereas a `bool` cannot and is fixed as just `true` or `false` over the whole lifecycle of the API.

* Detect and handle UNKNOWN in `ResponseVerifyVoteExtension`

Co-authored-by: William Banfield <4561443+williambanfield@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S:automerge Automatically merge PR when requirements pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ABCI++: change "accept/reject" booleans by enum all over the API

4 participants