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

impl StableDeref for LocatedSpan #65

Merged
merged 1 commit into from
Oct 16, 2020

Conversation

deepinthebuild
Copy link
Contributor

I can't implement this marker trait in my own code due to the orphan rule, and it's a very broadly used marker trait and should be safe to implement here. The added dependency doesn't have any transitive dependencies and only contains the trait definitions + simple marker impls.

@progval
Copy link
Collaborator

progval commented Oct 12, 2020

Hi,

Thanks for the PR!

Some requests for the dependencies:

  1. make stable_deref_trait an optional dependency, that isn't enabled by default (so we don't add a transitive deps to users of nom_locate unless they need it)
  2. set default-features = false on stable_deref_trait, so it doesn't include std and alloc, as nom_locate should also work without them

Also add a comment next to the trait impl (and optionally in Cargo.toml) to explain what this is about and why it's safe (a copy-paste of your commit message would be fine)

@progval progval self-assigned this Oct 12, 2020
LocatedSpan is largely just a wrapper around the contained type `T`, so
this marker trait is safe to implement whenever T already implements
StableDeref.
@deepinthebuild
Copy link
Contributor Author

Thanks for the reply. I have updated the PR with your requested changes.

@progval
Copy link
Collaborator

progval commented Oct 16, 2020

thanks!

@progval progval merged commit ffba5fc into fflorent:master Oct 16, 2020
@deepinthebuild
Copy link
Contributor Author

Would you be open to tagging a release that includes this PR and publishing it? If I understand correctly this would only be a minor version bump.

@progval
Copy link
Collaborator

progval commented Oct 17, 2020

I'd like to merge #66 first

@deepinthebuild
Copy link
Contributor Author

👋 Friendly ping to ask for cutting a new minor release.

@progval
Copy link
Collaborator

progval commented Oct 23, 2020

changelog updated, and I forwarded the ping

@fflorent
Copy link
Owner

fflorent commented Oct 24, 2020

Thanks for your work (both of you). I'll do this tomorrow or monday.

PS: I am very glad to see contributions and also proud and thankful that this project lives thanks to its community!

@fflorent
Copy link
Owner

wave Friendly ping to ask for cutting a new minor release.

Version 2.1.0 released! 🎉

https://crates.io/crates/nom_locate

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.

3 participants