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 2018 edition, bump MSRV to 1.41 and bump to v2.0.0 #41

Merged
merged 11 commits into from
Feb 28, 2023

Conversation

stevenroose
Copy link
Collaborator

Making a few other MRs obsolete, I think. This will have to wait for the 1.1.0 release first.

@stevenroose
Copy link
Collaborator Author

stevenroose commented Feb 23, 2023

We might want to add the unicode unpin, but I'm not sure what effect that will have. It's mostly pinned because that crate is known that have a different MSRV policy than we have and it has broken some of our crates in the past. So we can't rely on semver with them.

But like I mentioned before, since it's an unexposed dep, the pin shouldn't have any effect on dependent crates.

Copy link
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK 1b502ce

@stevenroose
Copy link
Collaborator Author

rebased and pushed

Copy link
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK ca7ea14

@stevenroose
Copy link
Collaborator Author

@tcharding I made some fixes and did the unicode-normalization fix. Could you review the commits (it wouldn't compile in my previous branch and you acked 😅 ) and could you also double check that it works in v1.41? I don't have rustup available on this machine, so I did some testing using Docker. I think it passes all tests in v1.41 with the rand feature tested.

Normally, using rustup, I used to use this script:

#!/bin/sh

set -e

MSRV="1.41.0"

CMD="rustup run ${MSRV}"

rm -f Cargo.lock
$CMD cargo generate-lockfile

$CMD cargo build --no-default-features
$CMD cargo test  --no-default-features
$CMD cargo build --no-default-features --features all-languages
$CMD cargo test  --no-default-features --features all-languages
$CMD cargo build --features rand,all-languages
$CMD cargo test  --features rand,all-languages

@stevenroose
Copy link
Collaborator Author

Let's merge and release the newly merged method in 1.x before merging this.

#44

@tcharding
Copy link
Member

That script runs, I made one change MSRV="1.41.1", which you had correct everywhere else.

Copy link
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK 40cc51c

@stevenroose
Copy link
Collaborator Author

Ok final rebase here after rebasing on v1.2.0 :)

As for 1.41.0/1 I wrote just 1.41 in most places. I did my tests with 1.41.0 and it seems to work.

@tcharding
Copy link
Member

As you prefer, in rust-bitcoin we always say 1.41.1 explicitly and I don't install the 1.41.0 toolchain myself.

@stevenroose
Copy link
Collaborator Author

There don't seem to be a significant difference, but I'll change it to 1.41.1 for consistency.

@stevenroose
Copy link
Collaborator Author

@tcharding test run and ack and should be good to go. I'm not happy with Travis disappearing and all my repos being left without CI :/

@stevenroose
Copy link
Collaborator Author

Rebased this MR on #45 to add CI, let's see. #45 passes

Copy link
Member

@tcharding tcharding left a comment

Choose a reason for hiding this comment

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

ACK f41bf17

@stevenroose stevenroose merged commit a30760b into master Feb 28, 2023
notmandatory added a commit to bitcoindevkit/bdk that referenced this pull request Mar 26, 2023
a4647cf Bump bip39 crate to v2.0.0 (Elias Rohrer)

Pull request description:

  <!-- You can erase any parts of this template not applicable to your Pull Request. -->

  ### Description

  The updated version of `rust-bip39` was just released. It also [bumped the pinned version](rust-bitcoin/rust-bip39#41) of `unicode-normalization`, which previously had lead to some incompatibilities.

  ### Notes to the reviewers

  <!-- In this section you can include notes directed to the reviewers, like explaining why some parts
  of the PR were done in a specific way -->

  ### Changelog notice

  <!-- Notice the release manager should include in the release tag message changelog -->
  <!-- See https://keepachangelog.com/en/1.0.0/ for examples -->

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

ACKs for top commit:
  rajarshimaitra:
    tACK a4647cf
  notmandatory:
    tACK a4647cf

Tree-SHA512: c13f2c5081cd1cf0477ed41717b09b4854651ee43434760b12a460c19657ec6f437d6401b0b6d32c8315491301d7c9848f0575d427f027adc8390d649fe898a9
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