-
Notifications
You must be signed in to change notification settings - Fork 255
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
Update dependencies #808
Update dependencies #808
Conversation
Closes #766.
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.
utACK with nonblocking suggestion
@@ -21,4 +21,3 @@ codegen-units = 1 | |||
[patch.crates-io] | |||
zcash_encoding = { path = "components/zcash_encoding" } | |||
zcash_note_encryption = { path = "components/zcash_note_encryption" } |
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.
Can we get rid of the path-based dependencies here? IIRC right now we're depending upon the released versions of these crates in zcash_primitives
and the other crates.
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.
When I remove that, we end up compiling zcash_note_encryption
twice: once for the copy in the workspace, and once for the released version (both of which end up in Cargo.lock
). It looks like only the released version gets used as a dependency, so maybe it's fine? In any case, changing this is sort of out-of-scope for this PR, because patch dependencies are only used in the local workspace; they don't affect downstream crate users.
@@ -15,7 +15,7 @@ categories = ["cryptography::cryptocurrencies", "encoding"] | |||
keywords = ["zcash", "address", "sapling", "unified"] | |||
|
|||
[dependencies] | |||
bech32 = "0.8" | |||
bech32 = "0.9" |
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.
AFAICT this dependency is not exposed, so we can cut a point release of zcash_address
at the same time we cut zcash_primitives 0.11
et al.
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.
This file is automatically generated, right?
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.
Yes; it is generated if protoc
is present in the path and the protobuf files are present. We don't include the protobuf files in the published releases (so that downstream users aren't forced into a hard dependency on protoc
), instead checking in the generated output.
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.
re-utACK f82866d
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.
utACK
No description provided.