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

Remove extern crates #1041

Merged
merged 4 commits into from Jul 20, 2022
Merged

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Jun 7, 2022

Do some clean up of extern crate statements now we have edition 2018.

4 separate but very similar patches, separated to ease review.

apoelstra added a commit to rust-bitcoin/bitcoin_hashes that referenced this pull request Jun 7, 2022
aba33d1 Fix macro usage in macros (Tobin C. Harding)

Pull request description:

  Currently we have macros that call other macros without a fully qualified path, this breaks some usage of the macros downstream because macro calls that should be opaque require importing.

  Fix macro usage in macros by using `$crate::` fully qualified path.

  Done in preparation for removing `extern crate` and `macro_use` in `rust-bitcoin` (since we have edition 2018). See rust-bitcoin/rust-bitcoin#1041 for demonstration of using the changes proposed here.

ACKs for top commit:
  apoelstra:
    ACK aba33d1

Tree-SHA512: 4c2f103baf7cfcb61e7d3bd333dca69258dd2e9c597ae93aea9b4c3987a5bfa799a09587f6ffbe106da8206de20a74264c7edfbda05ed927f2de700f4458b314
@Kixunil
Copy link
Collaborator

Kixunil commented Jun 16, 2022

Seeing all those internal_macros maybe we should have perma-unstable bitcoin_internal_macros crate so that we could stabilize bitcoin_hashes and bitcoin crates at some point.

@tcharding
Copy link
Member Author

tcharding commented Jun 16, 2022

Soon as v0.29.0 is out crate smashing is the next big task, I copied your comment over there so it doesn't get forgotten.

ref: #787

@tcharding tcharding changed the title Upgrade to bitcoin_hashes v0.11.0 and remove extern crates Remove extern crates Jun 28, 2022
@tcharding tcharding force-pushed the 06-06-extern-crates branch 3 times, most recently from a67a59c to c23c3ff Compare June 28, 2022 00:49
@tcharding tcharding marked this pull request as ready for review June 28, 2022 01:03
apoelstra
apoelstra previously approved these changes Jun 28, 2022
Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK c23c3ff

@tcharding tcharding marked this pull request as draft July 11, 2022 05:10
@tcharding tcharding marked this pull request as ready for review July 19, 2022 04:18
apoelstra
apoelstra previously approved these changes Jul 19, 2022
Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 7b50e82

sanket1729
sanket1729 previously approved these changes Jul 19, 2022
Copy link
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

utACK 7b50e82. But has conflicts

Now we have edition 2018 we do not need to use `macro_use` or `extern
crate`; `pub use` works with macros.

Remove the extern crate statement and replace it with a pub use
statement. Requires fixing up various imports statements and also
requires importing `sha256t_hash_newtype` macro directly instead of
using a qualified path to it.
Recently we removed the "unstable" feature, I missed the duplicate
`extern crate test` when doing so :(

Since we no longer have the "unstable" feature this line of code is
never compiled in.

Remove the unused ``extern crate test`, we have the correct line further
up the file `#[cfg(bench)] extern crate test;`.
Now we have edition 2018 we do not need to use `macro_use` or `extern
crate`; `pub use` works with macros. Notable exceptions are `alloc` and
`test`. Also leave the serde rename because touching it opens a can of
worms.
For internal macros used only in this crate we do not need to use
`macro_use` and pollute the top level namespace now that we have edition
2018. We can add a `pub(crate) use` statement to each and then path
imports work for the macros like normal types.
@sanket1729
Copy link
Member

utACK 21a1cc7

(21a1cc7) . Reviewed the range-diff from 7b50e82

Copy link
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

utACK 21a1cc7

Copy link
Member

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

ACK 21a1cc7

@apoelstra apoelstra merged commit 2256d46 into rust-bitcoin:master Jul 20, 2022
@tcharding tcharding deleted the 06-06-extern-crates branch July 20, 2022 22:24
ChallengeDev210 pushed a commit to ChallengeDev210/rust-bitcoin that referenced this pull request Aug 1, 2022
21a1cc7 Use pub(crate) for macros instead of macro_use (Tobin C. Harding)
23ee093 Remove extern crate (Tobin C. Harding)
da8b1b5 Remove unused extern crate test (Tobin C. Harding)
01a8cc6 Remove extern crate bitcoin_hashes (Tobin C. Harding)

Pull request description:

  Do some clean up of `extern crate` statements now we have edition 2018.

  4 separate but very similar patches, separated to ease review.

ACKs for top commit:
  sanket1729:
    utACK 21a1cc7
  apoelstra:
    ACK 21a1cc7

Tree-SHA512: fba33ed8fd261cc756dad8dd94f186a5b38aaf20cf31c3a83ad7633e7bb60a390681c39ebfd913e9e242fffed3b502491d067250d72ebfe666b4d03e93c8b945
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.

None yet

4 participants