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

Bump Bech32 to 0.7.2 #376

Closed
wants to merge 2 commits into from
Closed

Conversation

clarkmoody
Copy link
Member

Quick dependency bump, after fixing a logic error in the Bech32 library.

See rust-bitcoin/rust-bech32#41.

@codecov-io
Copy link

codecov-io commented Jan 3, 2020

Codecov Report

Merging #376 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #376   +/-   ##
=======================================
  Coverage   84.78%   84.78%           
=======================================
  Files          39       39           
  Lines        7221     7221           
=======================================
  Hits         6122     6122           
  Misses       1099     1099

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2caebc...f405745. Read the comment docs.

sgeisler
sgeisler previously approved these changes Jan 3, 2020
Copy link
Contributor

@sgeisler sgeisler left a comment

Choose a reason for hiding this comment

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

I reviewed rust-bitcoin/rust-bech32#41, LGTM.

@elichai
Copy link
Member

elichai commented Jan 3, 2020

There's no need to bump minor versions of dependencies.
According to cargo's semver minors are ignored anyway.

https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html

Just delete your cargo.lock and you should get the latest

@clarkmoody
Copy link
Member Author

@elichai 0.7.2 fixes an encoding bug in the Bech32 library, which should make its way into dependent projects. Do you think we should bump to 0.8 on Bech32?

@apoelstra
Copy link
Member

I think we've already merged some breaking changes and will need to backport this to get a minor release.

@elichai
Copy link
Member

elichai commented Jan 3, 2020

I don't understand.
If bech32 0.7.2 was released already then that's it, users of rust-bitcoin have it too.

Am I missing something or are you asking if you should yank 0.7.2 and release as 0.8?

@clarkmoody
Copy link
Member Author

I don't understand.
If bech32 0.7.2 was released already then that's it, users of rust-bitcoin have it too.

I don't think so. This lib specifies the dependency exactly. I don't think cargo will grab the patch version automatically.

bech32 = "0.7.1"

What you're describing would be

bech32 = "~0.7"

@apoelstra
Copy link
Member

= "0.7.1" should grab 0.7.2.

@clarkmoody
Copy link
Member Author

Ah, from the doc:

The string "0.1.12" is a semver version requirement. Since this string does not have any operators in it, it is interpreted the same way as if we had specified "^0.1.12", which is called a caret requirement.

So this should start getting picked up transparently?

@elichai
Copy link
Member

elichai commented Jan 3, 2020

Yes. As long as you don't have a cargo.lock this will get picked up transparently

@clarkmoody clarkmoody changed the title Bump Bech32 to 0.7.2 and library to 0.21.1 Bump Bech32 to 0.7.2 Jan 3, 2020
@clarkmoody
Copy link
Member Author

Ok, I've updated the PR to remove the library bump, as we can do that independently.

@apoelstra
Copy link
Member

#349 bumps this to 0.7.3. So I think this PR is unneeded now?

@clarkmoody
Copy link
Member Author

clarkmoody commented Jan 5, 2020 via email

@stevenroose
Copy link
Collaborator

I'm about to create a new PR for v0.23.0 that bumps other dependencies as well. I can include it if that's easier. Even though this is a backwards compatible change, it could be 0.22.1.

@clarkmoody
Copy link
Member Author

@stevenroose That works for me.

@clarkmoody
Copy link
Member Author

Fixed via #380

@clarkmoody clarkmoody closed this Jan 10, 2020
@clarkmoody clarkmoody deleted the bech32-0.7.2 branch January 10, 2020 21:09
Davidson-Souza pushed a commit to Davidson-Souza/rust-bitcoin that referenced this pull request Jul 12, 2023
…n; bump major version number

aa51638 update changelog for 0.22.0 (Andrew Poelstra)
d06dd20 update fuzzdummy API to match normal API (Andrew Poelstra)
f3d48a2 update "should terminate abnormally" test to trigger a different ARG_CHECK (Andrew Poelstra)
8294ea3 secp256k1-sys: update upstream library (Andrew Poelstra)
2932179 secp256k1-sys: update secp256k1.h.patch (Andrew Poelstra)

Pull request description:

  Should wait on merging until we get a minor release out with rust-bitcoin#382 and rust-bitcoin#376.

  May also want to bundle rust-bitcoin#380 with this?

ACKs for top commit:
  real-or-random:
    ACK rust-bitcoin/rust-secp256k1@aa51638 I can't judge if the feature set is meaningful but this release PR is fine

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

6 participants