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

Convert All Default Implementations on Enums to derive(Default) #3083

Merged
merged 3 commits into from
Oct 7, 2022

Conversation

killianc3
Copy link
Contributor

@killianc3 killianc3 commented Oct 7, 2022

Checklist

  • Run cargo clippy.
  • Run RUSTFLAGS=--cfg=web_sys_unstable_apis cargo clippy --target wasm32-unknown-unknown if applicable.
  • Add change to CHANGELOG.md. See simple instructions inside file.

Connections
Closes #3077.

Description
Since rust-lang/rust#101040, we can now mark enums as #[default], this replaces manual implementation where possible.

Testing
I ran the cargo test before and after the changes and nothing changed, so I guess nothing was broken because these are very simple changes.

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Code looks good, just need to change the msrv in the Readme and ci to be the latest needed for #[default]

@killianc3
Copy link
Contributor Author

It seems that the minimum supported version is now 1.62.0. (https://github.com/rust-lang/rust/blob/master/RELEASES.md#version-1620-2022-06-30)

@cwfitzgerald
Copy link
Member

Nice, not nearly as bad as I thought

@killianc3
Copy link
Contributor Author

Yes, it's not so bad and it seems to have passed the test now. Should we add an entry to the changelog to say that the msvr has changed ? If so, I don't really know how and what to add, any ideas ?

@cwfitzgerald
Copy link
Member

cwfitzgerald commented Oct 7, 2022

Look at last releases big announcements - I'd add a thing like that that just says what msrv we bumped to.

@killianc3
Copy link
Contributor Author

Thank you ! I think it's pretty understandable as is, what do you think ?

Copy link
Member

@cwfitzgerald cwfitzgerald left a comment

Choose a reason for hiding this comment

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

Thanks!

@cwfitzgerald cwfitzgerald enabled auto-merge (rebase) October 7, 2022 21:58
@cwfitzgerald cwfitzgerald disabled auto-merge October 7, 2022 21:58
@cwfitzgerald cwfitzgerald enabled auto-merge (rebase) October 7, 2022 21:59
@cwfitzgerald cwfitzgerald merged commit 6451d31 into gfx-rs:master Oct 7, 2022
@killianc3 killianc3 deleted the dev branch October 8, 2022 07:21
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.

Convert All Default Implementations on Enums to derive(Default) + #[default]
2 participants