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

Deprecate reexports of types that have been moved to other crates. #1672

Merged
merged 3 commits into from
Dec 30, 2024

Conversation

nuttycom
Copy link
Contributor

@nuttycom nuttycom commented Dec 20, 2024

The first commit of this PR removes the pub use reexports of types that have been moved to other crates and replaces the internal usage of these types with the types from their new crates. The second commit reintroduces those pub use reexports with deprecated pragmas.

Copy link

codecov bot commented Dec 20, 2024

Codecov Report

Attention: Patch coverage is 66.55290% with 98 lines in your changes missing coverage. Please review.

Project coverage is 52.26%. Comparing base (84fdb1f) to head (1855385).
Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
zcash_client_backend/src/data_api/testing/pool.rs 67.74% 30 Missing ⚠️
components/zcash_address/src/convert.rs 38.46% 8 Missing ⚠️
zcash_client_backend/src/data_api/wallet.rs 0.00% 7 Missing ⚠️
zcash_client_backend/src/data_api.rs 80.00% 6 Missing ⚠️
devtools/src/bin/inspect/address.rs 0.00% 5 Missing ⚠️
devtools/src/bin/inspect/context.rs 0.00% 4 Missing ⚠️
zcash_client_backend/src/fees/sapling.rs 0.00% 4 Missing ⚠️
zcash_client_backend/src/wallet.rs 55.55% 4 Missing ⚠️
zcash_client_sqlite/src/wallet.rs 25.00% 3 Missing ⚠️
zcash_client_sqlite/src/wallet/common.rs 57.14% 3 Missing ⚠️
... and 16 more
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1672   +/-   ##
=======================================
  Coverage   52.25%   52.26%           
=======================================
  Files         179      179           
  Lines       21376    21362   -14     
=======================================
- Hits        11171    11164    -7     
+ Misses      10205    10198    -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@nuttycom nuttycom force-pushed the deprecate_reexports branch 3 times, most recently from d97f306 to 0a9cfc7 Compare December 20, 2024 17:25
…ates.

This commit temporarily removes the reexported types to simplify the
removal process; the reexports will be reintroduced with deprecation
annotations in the subsequent commit.
@nuttycom nuttycom force-pushed the deprecate_reexports branch from 0a9cfc7 to 684097a Compare December 20, 2024 17:36
@nuttycom nuttycom force-pushed the deprecate_reexports branch from 684097a to 4b712f7 Compare December 30, 2024 17:32
Due to limitations described in
rust-lang/rust#30827, we can't just deprecate
the `pub use` stanzas; instead, we have to replace reexports with
new (deprecated) modules and public types which then internally reexport
the module internals or alias the reexported types, respectively.
@nuttycom nuttycom force-pushed the deprecate_reexports branch from 4b712f7 to 4bdf39c Compare December 30, 2024 17:38
str4d
str4d previously approved these changes Dec 30, 2024
Copy link
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

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

utACK 4bdf39c with nits (that will likely require a cargo fmt).

zcash_client_backend/src/data_api/wallet.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/error.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/lib.rs Outdated Show resolved Hide resolved
zcash_client_sqlite/src/lib.rs Outdated Show resolved Hide resolved
zcash_primitives/src/transaction/builder.rs Outdated Show resolved Hide resolved
zcash_primitives/src/transaction/mod.rs Outdated Show resolved Hide resolved
zcash_primitives/src/transaction/sighash.rs Outdated Show resolved Hide resolved
zcash_primitives/src/transaction/sighash_v4.rs Outdated Show resolved Hide resolved
zcash_primitives/src/transaction/txid.rs Outdated Show resolved Hide resolved
@nuttycom nuttycom force-pushed the deprecate_reexports branch from 2e01292 to 1855385 Compare December 30, 2024 19:18
Copy link
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

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

utACK 1855385

@nuttycom nuttycom merged commit 535c234 into zcash:main Dec 30, 2024
30 checks passed
@nuttycom nuttycom deleted the deprecate_reexports branch December 30, 2024 19:37
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.

2 participants