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

Edition 2018 clean up #113

Merged
merged 20 commits into from
Aug 23, 2019
Merged

Edition 2018 clean up #113

merged 20 commits into from
Aug 23, 2019

Conversation

Eirik0
Copy link
Contributor

@Eirik0 Eirik0 commented Aug 20, 2019

This PR runs cargo fix --edition and cargo fix --edition-idioms on all of the modules which needed it, and includes some warning cleanup along the way. The folders zcash_primitives/benches/... and pairing/benches/... were omitted when running the fix because they depend on unstable features, so it is possible that they no longer work. @ebfull suggested updating the benchmarks to use https://docs.rs/criterion/0.2.11/criterion/ which could be done as part of this PR, but might make more sense to be done as follow up.

Closes #97.

@Eirik0
Copy link
Contributor Author

Eirik0 commented Aug 20, 2019

There is a typo in one of the commit messages, and it looks like this will fail CI due needing a cargo fmt. This will be fixed shortly.

@Eirik0 Eirik0 changed the title Edition 2018 clean up [WIP] Edition 2018 clean up Aug 20, 2019
@Eirik0 Eirik0 force-pushed the edition-2018-clean-up branch from 56965a7 to 76795a9 Compare August 21, 2019 04:27
@@ -18,7 +18,7 @@ macro_rules! curve_impl {
}

impl ::std::fmt::Display for $affine {
fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
fn fmt(&self, f: &mut ::std::fmt::Formatter<'_>) -> ::std::fmt::Result {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: In pairing I did a find and replace on ::std::fmt::Formatter/::std::fmt::Formatter<'_>. cargo fix --edition-idioms would fail here and try to double add the <'_>.

@Eirik0 Eirik0 changed the title [WIP] Edition 2018 clean up Edition 2018 clean up Aug 21, 2019
@Eirik0 Eirik0 requested review from str4d and ebfull August 21, 2019 05:44
Copy link
Contributor

@str4d str4d left a comment

Choose a reason for hiding this comment

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

ACK mod comments, but zcash_proofs has not been editionized.

zcash_primitives/src/jubjub/fs.rs Outdated Show resolved Hide resolved
zcash_primitives/src/jubjub/fs.rs Outdated Show resolved Hide resolved

#[cfg(feature = "multicore")]
extern crate crossbeam;
#[cfg(feature = "multicore")]
extern crate futures_cpupool;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this multicore-conditional extern crate disappear, but not the other two?

Copy link
Contributor

Choose a reason for hiding this comment

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

extern crate hex_literal stays as expected because of the #[macro_use].

@@ -23,11 +21,9 @@ extern crate hex_literal;
#[cfg(test)]
extern crate rand;

#[cfg(test)]
extern crate rand_xorshift;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this test-conditional extern crate disappear, but not the extern crate rand?

@Eirik0
Copy link
Contributor Author

Eirik0 commented Aug 21, 2019

ACK mod comments, but zcash_proofs has not been editionized.

There were no changes when running cargo fix --edition, so I guess I didn't do the subsequent steps. This is now fixed.

@str4d str4d added this to the v0.1.0 milestone Aug 22, 2019
@str4d str4d mentioned this pull request Aug 23, 2019
@ebfull ebfull mentioned this pull request Aug 23, 2019
@str4d str4d merged commit ad33798 into zcash:master Aug 23, 2019
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.

Migrate all crates to 2018 edition
3 participants