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

Move bip158 module to crate root #1211

Merged
merged 2 commits into from Sep 6, 2022

Conversation

tcharding
Copy link
Member

We are attempting to flatten the util module.

Move the bip158 module to the crate root out of util.

Currently src/util/ is ignored by the formatter so this move requires bip158 module to be formatted. Formatting is done as a separate patch so reviewers can run cargo +nightly fmt and compare the diffs if so desired.

@apoelstra
Copy link
Member

The new match-arm formatting looks kinda bad to me but I won't die on this hill.

apoelstra
apoelstra previously approved these changes Aug 24, 2022
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.

ACK bcc7afc

@tcharding
Copy link
Member Author

Now you mention it, I tend to agree. So as not to open up a can of worms right now perhaps we can leave it like this and come back to it at a later time?

@apoelstra
Copy link
Member

If you mean, leaving this file blacklisted by cargo fmt, I'm fine with that. But I'm also fine with the new crappy formatting :).

@tcharding
Copy link
Member Author

tcharding commented Aug 25, 2022

I'll leave the new formatting, I'm pretty sure this was one of the config changes introduced during the "less vertical lines is better" debate.

We are attempting to flatten the `util` module; move the `bip158` module
to the crate root out of `util`.

Currently `src/util/` is ignored by the formatter so this move causes
the `bip32` module to be formatted.
Run the formmater on the newly moved `bip158` module. No changes other
than those introduced by `cargo +nightly fmt`.
@tcharding
Copy link
Member Author

Rebase to pick up recent fuzzer change.

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.

ACK 01161e6

@tcharding tcharding added this to the 0.30.0 milestone Aug 26, 2022
@tcharding tcharding added minor API Change This PR should get a minor version bump 1.0 Issues and PRs required or helping to stabilize the API aggregate release notes mention labels Aug 26, 2022
Copy link
Member

@sanket1729 sanket1729 left a comment

Choose a reason for hiding this comment

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

ACK 01161e6

@apoelstra apoelstra merged commit 2bb3552 into rust-bitcoin:master Sep 6, 2022
@tcharding tcharding deleted the 08-24-rustfmt-bip158 branch September 12, 2022 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.0 Issues and PRs required or helping to stabilize the API aggregate release notes mention minor API Change This PR should get a minor version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants