-
Notifications
You must be signed in to change notification settings - Fork 253
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
Disable the default features of the tonic
and tonic-build
crates to allow downstream consumers to build in Wasm
#1270
Disable the default features of the tonic
and tonic-build
crates to allow downstream consumers to build in Wasm
#1270
Conversation
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.
Thanks for the PR!
Also, in addition to making the above change, add the following change to diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 6263019a6..ebf418cfe 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -195,7 +195,7 @@ jobs:
run: cargo add --no-default-features --path ../crates/zcash_proofs
- name: Add zcash_client_backend as a dependency of the synthetic crate
working-directory: ./ci-build
- run: cargo add --no-default-features --path ../crates/zcash_client_backend
+ run: cargo add --features lightwalletd-tonic --path ../crates/zcash_client_backend
- name: Copy pinned dependencies into synthetic crate
run: cp crates/Cargo.lock ci-build/
- name: Add target |
Thanks for the review @str4d ! I moved the impl of I think this makes more sense as it it feels a little weird to have the API change depending on the build target. Instead builds will fail if the feature is enabled and then Wasm builds can disable it to have it work. I think more useful information is communicated this way. |
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.
Reviewed 61447d9
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1270 +/- ##
==========================================
- Coverage 63.55% 63.42% -0.13%
==========================================
Files 121 121
Lines 13661 13705 +44
==========================================
+ Hits 8682 8693 +11
- Misses 4979 5012 +33 ☔ View full report in Codecov by Sentry. |
Closes #1269
Note this regenerates the code-generated
proto/service.rs
file and the newer version is included in this PR. This is a minor change to thezcash_client_backend
public API as these are exposed.