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

pex: remove legacy proto messages #7147

Merged
merged 10 commits into from
Oct 22, 2021
Merged

pex: remove legacy proto messages #7147

merged 10 commits into from
Oct 22, 2021

Conversation

cmwaters
Copy link
Contributor

This PR implements the proto changes made in tendermint/spec#352, removing the legacy messages that were used in the pex reactor.

.gitignore Outdated
@@ -46,3 +46,4 @@ test/fuzz/**/corpus
test/fuzz/**/crashers
test/fuzz/**/suppressions
test/fuzz/**/*.zip
*.proto
Copy link
Contributor

Choose a reason for hiding this comment

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

We still have a bunch of other proto files in the repo, do we really want this at the top level?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right good catch. I will make it more granular

set -eo pipefail

VERS=master
Copy link
Contributor

Choose a reason for hiding this comment

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

Was the goal here to allow this to be set from the environment? If not, we don't really need the conditional stuff. Otherwise consider maybe:

Suggested change
VERS=master
: ${VERS:=master}

…which would allow, e.g., VERS=blah ./scripts/protocgen.sh.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It wasn't like that before, but this makes sense 👍

@tychoish tychoish added the S:automerge Automatically merge PR when requirements pass label Oct 22, 2021
@mergify mergify bot merged commit 68ca65f into master Oct 22, 2021
@mergify mergify bot deleted the callum/remove-legacy-pex branch October 22, 2021 12:07
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.

None yet

3 participants