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

Use nom 7.0.0-alpha1 to avoid issues with dependencies. #3

Merged
merged 1 commit into from
Jul 16, 2021
Merged

Use nom 7.0.0-alpha1 to avoid issues with dependencies. #3

merged 1 commit into from
Jul 16, 2021

Conversation

damirka
Copy link
Contributor

@damirka damirka commented Jul 15, 2021

Hey! We use this crate and abnf crate in Leo language: https://github.com/AleoHQ/leo


Recently, there has been inconsistency across community due to updates in nom, bitvec and funty.
Bumping to 0.7.0-alpha1 solves the problem and makes it work. I do understand that it's an alpha version and might not be considered safe, but that's the only solution for now.

Also adds Cargo.lock to the git to maintain best practices.

@duesee
Copy link
Owner

duesee commented Jul 15, 2021

Hello! Thank you very much for your PR! Basically, I am fine using an alpha version of nom if this solves real-world issues. However, I would also like to understand what do you mean with the inconsistencies. I would be very glad if you could explain!

From my understanding we do not want to include the Cargo.lock file, see https://doc.rust-lang.org/cargo/faq.html#why-do-binaries-have-cargolock-in-version-control-but-not-libraries.

Could you also rename the commits, such that they match the changes? Otherwise it looks like you bump nom to 0.6.5.

It would also be nice to increase the abnf-core version some more. I like to keep it below 1.0.0 for now, so 0.5.0 would be more appropriate.

What do you think?

@damirka
Copy link
Contributor Author

damirka commented Jul 15, 2021

@duesee here are two links for a context:

From my understanding we do not want to include the Cargo.lock file, see https://doc.rust-lang.org/cargo/faq.html#why-do-binaries-have-cargolock-in-version-control-but-not-libraries.

Thanks! Didn't know. I will return .gitignore for a lock file.

Could you also rename the commits, such that they match the changes? Otherwise it looks like you bump nom to 0.6.5.

Will do.

It would also be nice to increase the abnf-core version some more. I like to keep it below 1.0.0 for now, so 0.5.0 would be more appropriate.

Sure, will update.

@damirka damirka changed the title Bump nom to 0.7.0-alpha1, add Cargo.lock Bump nom to 0.7.0-alpha1, update version to 0.5.0 Jul 15, 2021
@duesee duesee changed the title Bump nom to 0.7.0-alpha1, update version to 0.5.0 Use nom 7.0.0-alpha1 to avoid issues with dependencies. Jul 16, 2021
@duesee
Copy link
Owner

duesee commented Jul 16, 2021

Thank you! One more nit: could you make it so that the .gitignore file is not part of the commit and also set the commit message to "Use nom 7.0.0-alpha1 to avoid issues with dependencies"? I am happy if this solves a real issue, but also want to make sure that the commit history is comprehensible.

Btw, I tried to do the changes for you by editing the commit, but somehow I were not able to keep you as the commit author. So it would be better if you could do this:-)

@damirka
Copy link
Contributor Author

damirka commented Jul 16, 2021

@duesee Done. If that's your last request, then ready to merge. 👍

@duesee duesee merged commit 11ac322 into duesee:main Jul 16, 2021
@duesee
Copy link
Owner

duesee commented Jul 16, 2021

Thanks!

@damirka damirka deleted the bump-nom-crate branch July 16, 2021 13:31
@duesee
Copy link
Owner

duesee commented Jul 16, 2021

New version published to crates.io https://crates.io/crates/abnf-core/0.5.0.

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