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

Remove num-traits altogether #957

Merged
merged 1 commit into from
Dec 31, 2019
Merged

Conversation

dmitmel
Copy link
Contributor

@dmitmel dmitmel commented Dec 31, 2019

I replaced every occurrence of FromPrimitive with TryFrom with () as the error type.

I also noticed a recurring pattern of casting C enums to uints and then converting uints to Rust enums, perhaps this can be avoided by adding From/TryFrom traits for converting C enums directly to Rust ones? And some TryFrom implementations can be converted to From by assuming default values, e.g. PixelFormatEnum::Unknown when the pixel format is unknown.

@Cobrand
Copy link
Member

Cobrand commented Dec 31, 2019

This crate was originally developped when rust didn't have a 1.0 yet, so there are for sure some stuff we can improve, notably with the TryFrom traits and all. In the meantime, thanks for the PR!

@Cobrand Cobrand merged commit 2490118 into Rust-SDL2:master Dec 31, 2019
@dmitmel
Copy link
Contributor Author

dmitmel commented Dec 31, 2019

This crate was originally developped when rust didn't have a 1.0 yet

I see.

What do you think about my other suggestions? I can make them in a PR as well.

@Cobrand
Copy link
Member

Cobrand commented Jan 2, 2020

I think your suggestion is a good one (but perhaps you'll encounter issues you didn't expect while changing that, who knows...); but I don't think it is an urgent one. Feel free to drop a PR if you're up for it!

sypwex pushed a commit to sypwex/rust-sdl2 that referenced this pull request Jun 2, 2024
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