8000 Pin serde and re-export bech32 by elichai · Pull Request #316 · rust-bitcoin/rust-bitcoin · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Pin serde and re-export bech32 #316

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 16, 2019
Merged

Pin serde and re-export bech32 #316

merged 2 commits into from
Aug 16, 2019

Conversation

elichai
Copy link
Member
@elichai elichai commented Aug 16, 2019 •
edited
Loading

As per IRC discussion:

12:43 < andytoshi> oh, damn, i thought we re-exported bech32
12:45 < elichai2> no it was only hashes and secp iirc
12:47 < andytoshi> if someone is so inclined as to export bech32, that'd be cool..
12:47 < andytoshi> and bump to 0.19.1
12:49 < elichai2> We have a bigger problem
12:49 < elichai2> "Minimum supported rustc version is 1.31"
12:49 < andytoshi> where's that?
12:54 < elichai2> ok. apperantly it's serde-derive fault
12:54 < elichai2> he published a new minor version while bumping a major proc-macro2
12:54 < elichai2> https://github.com/serde-rs/serde/blob/v1.0.98/serde_derive/Cargo.toml
12:54 < elichai2> https://github.com/serde-rs/serde/blob/v1.0.99/serde_derive/Cargo.toml
13:01 < andytoshi> patch version even
13:02 < andytoshi> w/e we can just pi
13:02 < dongcarl> :man-facepalming:
13:02 < andytoshi> but 
13:04 < elichai2> yep
13:04 < elichai2> we'll see what dtolnay will say but I can probably guess.... https://github.com/serde-rs/serde/issues/1602
13:06 < elichai2> i'll open a PR pinning, exporting and bump to 0.19.1. wanna yank 0.19.0?
13:07 < andytoshi> all old versions are affected

cc serde-rs/serde#1602

@codecov-io
Copy link

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #316   +/-   ##
=======================================
  Coverage   81.85%   81.85%           
=======================================
  Files          38       38           
  Lines        6951     6951           
=======================================
  Hits         5690     5690           
  Misses       1261     1261
Impacted Files Coverage Δ
src/lib.rs 100% <ø> (ø) ⬆️

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 fbc8510...0153061. Read the comment docs.

@apoelstra
Copy link
Member

What will happen to the serde version that users get when they use this PR and have other dependencies on serde?

@apoelstra
Copy link
Member

Can we publicly export serde?

@elichai
Copy link
Member Author
elichai commented Aug 16, 2019

Hopefully no other dependency will pin a serde version and then it will resolve to the versions pinned here

Re exporting is mostly needed for multiple major versions

apoelstra
apoelstra approved these changes Aug 16, 2019
View reviewed changes
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.

ok wfm

@apoelstra apoelstra merged commit 4556c9c into rust-bitcoin:master Aug 16, 2019
@stevenroose
Copy link
Collaborator

I don't think reexporting makes much sense in the case of serde. It's seldomly used by end users directly and if so, they just use the derive feature and I don't think you can derive from an exported crate out of ours...

@apoelstra
Copy link
Member

Yeah, probably not

@dtolnay dtolnay mentioned this pull request Aug 17, 2019
Closed
@elichai elichai deleted the 2019-08-pin-export branch August 18, 2019 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stevenroose stevenroose stevenroose approved these changes

@apoelstra apoelstra apoelstra approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@elichai @codecov-io @apoelstra @stevenroose
0