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

refactor: remove unnecessary u32 casts #91

Closed
wants to merge 1 commit into from
Closed

refactor: remove unnecessary u32 casts #91

wants to merge 1 commit into from

Conversation

hamirmahal
Copy link

@hamirmahal hamirmahal commented May 22, 2024

This change is Reviewable

Copy link
Author

@hamirmahal hamirmahal left a comment

Choose a reason for hiding this comment

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

It might make sense to wait until GitHub Actions are set up, to be more certain these changes don't break anything.

@faern
Copy link
Member

faern commented May 29, 2024

Thanks for the PR. We'll wait until #93 and #89 are merged. That way we'll know if the updated bindings changes anything and if the CI catch anything weird.

@faern
Copy link
Member

faern commented May 29, 2024

The new bindings have been merged. So if you want to you can rebase this on top of main and try it out and see if it works locally for you and if the automatic tests pass.

@faern
Copy link
Member

faern commented May 29, 2024

All the CI stuff has been merged! Feel free to rebase again and I'll merge if it passes.

@hamirmahal
Copy link
Author

Nice! I went ahead and updated this branch, although it looks like this PR needs maintainer approval to run the CI checks.

@faern
Copy link
Member

faern commented May 29, 2024

I was a bit afraid it would turn out like this with the new bindings. The new bindings generate actual Rust enums for the anonymous C enums, not just ints. This has the benefit that related constants are grouped and a bit stricter typed. But the downside is that they must be cast to be used as integers again.

I don't think we can merge this?

@hamirmahal
Copy link
Author

Ah, I understand. I'll go ahead and close this.

@hamirmahal hamirmahal closed this May 29, 2024
@hamirmahal hamirmahal deleted the refactor/remove-unnecessary-u32-casts branch May 29, 2024 21:31
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