Skip to content

Conversation

@james7132
Copy link
Contributor

@james7132 james7132 commented Sep 21, 2025

Serde recently split out the trait definitions from the top level serde crate. This crate does not rely on the derive macro, so it should be effective to depend on serde_core instead of serde. This doesn't impact the compilation times of bitflags itself, but allows it to parallelize with serde-derive and potentially other crates like serde-json that also do not rely on the derive macro, unblocking other crates reliant on bitflags to do so as well. Done in #467.

This PR fully qualifies the path of the use of Result in the generated macro code.

Copy link
Member

@KodrAus KodrAus left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @james7132! I think we just need to update the serde_core version to one that's actually published, but otherwise this looks good to me.

@james7132 james7132 requested a review from KodrAus October 3, 2025 09:11
@james7132 james7132 requested a review from KodrAus October 3, 2025 23:55
Copy link
Member

@KodrAus KodrAus left a comment

Choose a reason for hiding this comment

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

Just spotted one more nitpick, then this should be good to go!

@KodrAus
Copy link
Member

KodrAus commented Oct 19, 2025

I had a look at those build failures and fixed this up in #467.

If you'd like to scope this PR back to just the $crate::__private::core::result::Result::Ok change I think that's also a great fix. Thanks again for working on this @james7132

@james7132
Copy link
Contributor Author

Currently on vacation, will do so sometime next week.

@james7132 james7132 changed the title Use serde_core instead of serde Fix use of Result in macro output Dec 7, 2025
@james7132
Copy link
Contributor Author

@KodrAus done!

Copy link
Member

@KodrAus KodrAus left a comment

Choose a reason for hiding this comment

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

Thanks @james7132!

@KodrAus KodrAus merged commit 90488e5 into bitflags:main Dec 9, 2025
10 checks passed
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