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

Enable edition 2018 #57

Merged
merged 7 commits into from
May 13, 2022
Merged

Conversation

tcharding
Copy link
Member

@tcharding tcharding commented Nov 22, 2021

Update the MSRV to Rust 1.41.1 and enable edition 2018.

Should not be merged without rust-bitcoin organization-wide planning on how to go about this upgrade.

Leaving as draft until rust-bitcoin Taproot release is done.

The Clippy fixes are needed to make the update complete (because we run Clippy in CI in rust-bech32), hence this should be squashed into a single commit. Leaving it separate for the first rounds of review.

Discussion: rust-bitcoin/rust-bitcoin#510 (comment)

@tcharding tcharding marked this pull request as draft November 22, 2021 03:48
@apoelstra
Copy link
Member

concept ACK from me -- though I think we should wait until after we've done the Taproot release for rust-bitcoin so that we know we're ready to increase MSRVs across the board.

@tcharding
Copy link
Member Author

Yep, I'm with you. I'll convert this to non-draft PR after release of rust-bitcoin 0.28. Thanks.

@tcharding tcharding force-pushed the edition-2018 branch 5 times, most recently from fe8c7a7 to 1676bc1 Compare January 15, 2022 02:14
@tcharding tcharding mentioned this pull request Jan 15, 2022
It seems we have consensus within the rust-bitcoin github organisation
on bumping the MSRV to 1.41.1, update CI job and the README to reflect
this.
Add `edition = 2018` to the manifest file. No further changes were
necessary.
Clippy emits:

  error: this expression borrows a reference (`&str`) that is
  immediately dereferenced by the compiler

As suggested, remove the unneeded reference.
Clippy emits:

 warning: long literal lacking separators

As suggested, use separators.
Clippy emits:

 warning: manual `!RangeInclusive::contains` implementation

Use the code snippets suggested by clippy.
Clippy emits:

 warning: very complex type used. Consider factoring parts into `type` definitions

This code is in a unit test, its clear enough for the purpose.

Configure clippy to allow type_complexity
We just bumped the MSRV, this is a major change but since we are pre 1.0
we just have to bump the minor version number.

Bump version from current `0.8.1` to `0.9.0`.
@tcharding tcharding marked this pull request as ready for review April 21, 2022 02:23
apoelstra added a commit to rust-bitcoin/bitcoin_hashes that referenced this pull request May 3, 2022
abb7c80 Bump version 0.10.0 -> 0.11.0 (Tobin Harding)
b270023 Update to use edition 2018 (Tobin Harding)
e762962 Update MSRV in CI and Readme from 1.29 to 1.41.1 (Tobin Harding)
3602c75 Remove trailing whitespace (Tobin Harding)

Pull request description:

  Update the MSRV to Rustn 1.41.1 and enable edition 2018.

  Should not be merged without rust-bitcoin organization-wide planning on how to go about this upgrade.

  Discussion: rust-bitcoin/rust-bitcoin#510 (comment)

  This one is a bit more involved than the same PRs for [rust-bech32](rust-bitcoin/rust-bech32#57) or [rust-bitcoinconcensus](rust-bitcoin/rust-bitcoinconsensus#34).

  The commit message of patch 3:
  ```
  Update to use edition 2018

  Add `edition = "2018"` to the minifest file. In order to get the
  codebase to build cleanly do:

  - Remove usage of `use Hash as HashTrait`, instead use `impl crate::Hash for Hash` and `use Hash as _`.
  - Same for HashEngine (remove EngineTrait).
  - Add `crate::` to import statements and group same level (only did this for crate imports, the rest can wait for rustfmt :)
  - Make test imports uniform, elect to _not_ use `super::*` because it seems cleaner, we are always importing the module we are testing and the same set of traits in each `test` module. Can change if requested.
  ```

  Thanks

ACKs for top commit:
  apoelstra:
    ACK abb7c80

Tree-SHA512: 6e99235075a12a82bc2bb032411eb7d022c650e5288bd1a2891b3d863e093ad9398525c1fba41d5e3fdcb194fcf93b00c6f59ad7681f5404eaeae73f08af2278
@tcharding
Copy link
Member Author

cc @clarkmoody, are you ok with this one please?

Copy link
Member

@clarkmoody clarkmoody left a comment

Choose a reason for hiding this comment

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

LGTM :shipit:

@clarkmoody clarkmoody merged commit 9a57c97 into rust-bitcoin:master May 13, 2022
@clarkmoody
Copy link
Member

Released 0.9.0 to crates.io

@jkczyz
Copy link
Contributor

jkczyz commented Jun 24, 2022

Looks like the repo is missing a tag for 0.9.0.

@clarkmoody
Copy link
Member

Looks like the repo is missing a tag for 0.9.0.

Tagged v0.9.0.

@tcharding tcharding deleted the edition-2018 branch June 29, 2022 02:29
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