-
Notifications
You must be signed in to change notification settings - Fork 732
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
Edition 2018 #983
Edition 2018 #983
Conversation
715f510
to
85afd86
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 85afd86
This all looks good to me.
Use cargo to upgrade from edition 2015 to edition 2018. cargo fix --edition No manual changes made. The result of the command above is just to fix all the use statements (add `crate::`) and fix the fully qualified path formats i.e., `::Foo` -> `crate::Foo`.
Add 'edition = "2018"' to the manifest and do a bunch of manual path fixups (use statements and fully qualified paths).
Rebased |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-ACK 9f0c687
Thanks for the reviews @dunxen! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-ACK 9f0c687
Not much interest in this one, I'm surprised. I'm personally pretty excited to get edition 2018 into the rust-bitcoin stack and the improvements that can be done on top of it. Can we get this one in crew? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 9f0c687,
my cargo fix --edition
didn't produce exactly dca0d67, but differences are minimal
--- a/src/blockdata/witness.rs
+++ b/src/blockdata/witness.rs
@@ -302,12 +302,12 @@ impl<'de> serde::Deserialize<'de> for Witness {
#[cfg(test)]
mod test {
- use crate::blockdata::transaction::EcdsaSighashType;
- use crate::blockdata::witness::Witness;
+ use super::*;
+
use crate::consensus::{deserialize, serialize};
use crate::hashes::hex::{FromHex, ToHex};
use crate::Transaction;
- use secp256k1::ecdsa;
+ use crate::secp256k1::ecdsa;
Thanks @RCasatta! |
Nice, let's do it! |
Likely to cause rebase issues but very simple ones. (And actually maybe not, since it mostly only touches import statements..) |
BOOM! |
9f0c687 Enable edition 2018 (Tobin C. Harding) dca0d67 Fix in preparation for next edition (Tobin C. Harding) Pull request description: This PR supersedes #635 at the permission of @Kixunil in the thread of that PR. Do a minimal set of changes to enable edition 2018. Patch 1 is the biggest change set, it is done with `cargo`, no other manual changes are included in patch 1. It can verified by running `cargo fix --edition` on master and checking the diffs are the same. Patch 2 enables 2018 and includes all the manual changes required to get the code to build (with _all_ the feature combinations :) ACKs for top commit: dunxen: re-ACK 9f0c687 apoelstra: re-ACK 9f0c687 RCasatta: ACK 9f0c687, Tree-SHA512: 7c23554adb4c1dd932af1e80f04397bad0418b4fdae2d0fe243c3f19ba1169686a386bffd38a3c02871c7544b5a7bc8af525b50617a2695726408c091700f081
This PR supersedes #635 at the permission of @Kixunil in the thread of that PR.
Do a minimal set of changes to enable edition 2018. Patch 1 is the biggest change set, it is done with
cargo
, no other manual changes are included in patch 1. It can verified by runningcargo fix --edition
on master and checking the diffs are the same.Patch 2 enables 2018 and includes all the manual changes required to get the code to build (with all the feature combinations :)