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

Update nom to 7.0.0 #102

Merged
merged 1 commit into from
Oct 24, 2021
Merged

Conversation

xd009642
Copy link

Motivation: Nom 6 used a version of bitvec using the old (and bad) way of ignoring code via tarpaulin #[cfg_attr(tarpaulin,skip)], unfortunately having this in a users dependency tree means you need to use an older version of tarpaulin or run --avoid-cfg-tarpaulin and limits a users ability to filter out code in coverage runs.

This PR updates nom to 7.0.0, it would also be nice for affected tarpaulin users if there could be a new hdrhistogram release after this is merged 🙏

Some other benefits mentioned in the nom changelog, faster float parsing, better compile times, and slimmer dependencies as bitvec and regex code are moved into separate nom crates to further reduce binary size and compilation time.

Copy link
Collaborator

@jonhoo jonhoo left a comment

Choose a reason for hiding this comment

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

I'm amazed that this just works with no other modifications. Nice!

Also double-checked that we don't accidentally expose nom in any public signatures.

@jonhoo
Copy link
Collaborator

jonhoo commented Oct 24, 2021

Ah, it's because CI isn't running since we moved the repo into the HdrHistogram org. I'll get on fixing that in #103. In the meantime, let me give running the tests for this CR a shot.

@jonhoo
Copy link
Collaborator

jonhoo commented Oct 24, 2021

Checked that tests pass locally, so merging!

@jonhoo jonhoo merged commit 36e9f6a into HdrHistogram:master Oct 24, 2021
@jonhoo
Copy link
Collaborator

jonhoo commented Oct 24, 2021

Released as 7.4.0 🎉

@xd009642
Copy link
Author

Awesome thanks 🎉 and yeah as long as you weren't using nom macros or anything moved to another crate (bitvec/regex) the 6->7 upgrade is often that simple 😁

@jonhoo
Copy link
Collaborator

jonhoo commented Oct 24, 2021

No, thank you!

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.

2 participants