Skip to content

Fix for rust commit 3dcd2157403163789aaf21a9ab3c4d30a7c6494d 'Switch to ...#60

Merged
lifthrasiir merged 2 commits intolifthrasiir:masterfrom
CraZySacX:master
Nov 18, 2014
Merged

Fix for rust commit 3dcd2157403163789aaf21a9ab3c4d30a7c6494d 'Switch to ...#60
lifthrasiir merged 2 commits intolifthrasiir:masterfrom
CraZySacX:master

Conversation

@CraZySacX
Copy link
Contributor

...purely namespaced enums'

src/types.rs Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we take the opportunity to rename the variants, instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. What should they be renamed as?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove the Decode and Encode prefixes on the variant names, but don’t re-export them. Take advantage on the new language feature! @lifthrasiir, what do you think? By the way, the language change is rust-lang/rust#18973

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So you are suggesting to remove the re-export, and use the fully qualified name elsewhere. So, DecoderTrap::Strict for example after the rename. Just making sure I'm thinking the same as you.

Copy link
Owner

Choose a reason for hiding this comment

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

I have no problem with renaming them. See the below for other comments.

@lifthrasiir
Copy link
Owner

Ideally a proper deprecation notice (#[deprecated] pub use self::DecodeTrap::Strict as DecodeStrict; etc.) might be desirable but it does not work currently (rust-lang/rust#15702). As long as we still have a small user base (haha) I think it's fine. I'd like to accept the updated PR.

lifthrasiir added a commit that referenced this pull request Nov 18, 2014
Switch to purely namespaced enums (not yet builds on nightly)
@lifthrasiir lifthrasiir merged commit 1466de1 into lifthrasiir:master Nov 18, 2014
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