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

Relax the semver requirement for protobuf #193

Merged
merged 1 commit into from
Oct 29, 2020
Merged

Conversation

clintfred
Copy link
Contributor

@clintfred clintfred commented Oct 26, 2020

This will allow downstream consumers to choose their version of protobuf without causing build issues.

I realized this while looking at https:/tikv/rust-prometheus/blob/master/Cargo.toml

I tested this by git reseting logdriver to an old revision that used protobuf 2.14. It compiled file. It also compiled fine with a dependency of 2.17.

There might still be problems in upgrading to newer version because we have custom build.rs logic for the proto generation, but at least cargo will be able to successfully select a version.

If we merge this, I think we can close #188

This will allow downstream consumers to choose their version of protobuf without causing build issues.
@clintfred clintfred changed the title Relax the semver requirement for ironoxide Relax the semver requirement for protobuf Oct 27, 2020
Copy link
Member

@giarc3 giarc3 left a comment

Choose a reason for hiding this comment

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

My opinion is that ^ beats ~ for any dependency that we trust to conform to semver.
Your change is equivalent to ^2, which might be a little more clear? But maybe not.

@coltfred
Copy link
Member

Seems entirely reasonable to me.

@BobWall23 BobWall23 merged commit 69b5c7c into master Oct 29, 2020
@BobWall23 BobWall23 deleted the relax-protobuf branch October 29, 2020 21:24
clintfred added a commit that referenced this pull request Dec 17, 2020
- [[#183](#183)]
  - Update to rust-protobuf 2.17
- [[#193](#193)]
  - Relax rust-protobuf dependency requirement. This should allow downstream consumers more freedom in what rust-protobuf version they are using.
- [[#196](#196)]
  - Add group encrypt benchmarks
- Various non-breaking dependency updates
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.

4 participants