-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add optional bincode dependency #28
Conversation
Cargo.toml
Outdated
[dependencies] | ||
vob = { version=">=3.0.2", features=["serde"] } | ||
packedvec = { version="1.0", features=["serde"] } | ||
vob = { version=">=3.0.4" } |
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.
Do we need the >=
? If so, we need it on packedvec
too I suppose? Or we need it on neither, and just specifying the version number is equivalent to >=
?
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.
I was just mirroring how it was, and was hoping we could remove it.
Cargo.toml
Outdated
[dependencies] | ||
vob = { version=">=3.0.2", features=["serde"] } | ||
packedvec = { version="1.0", features=["serde"] } | ||
vob = { version=">=3.0.4" } |
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.
I made this one a draft primarily because I was wondering about this "version = ">=3.0.4"
.
Perhaps I am blind, but I couldn't find anything in the cargo docs saying that this >=
implies within semver range?
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.
Aha, I think >=
means "this or later versions even with major changes". I think we should drop the >=
so we are compatible with new minor, but not major, versions.
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.
Will gladly go ahead and remove it.
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.
Fixed in ad58740 will probably want to squash.
Please squash. |
Squashed. |
No description provided.