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
2.0.0 changelog appears incomplete #310
Comments
On a related note (though could probably be asked in a distinct issue): how do we now derive features on the inner type? I'm used to deriving for example For example |
Hi @MarijnS95 👋 Thanks for pointing this all out and for digging into it! We should update our release notes to make upgrading from As for how to derive traits on the inner type, that's no longer possible from outside of What we should do now is add feature flags to |
@KodrAus thanks! Looking forward to the release note updates! Indeed, that's what other crates like
We've seen crates like |
I just finished migrating one of my projects to v2 and, in addition to the issues mentioned here, one problem I ran into is lost support for multi-bit masks. This was especially surprising when serde serialize/deserialize round-trip produced a rather confusing test failure: #[test]
fn bit_mask() {
bitflags::bitflags! {
#[derive(Clone, Copy, Debug, Eq, PartialEq, serde::Deserialize, serde::Serialize)]
#[repr(transparent)]
#[serde(transparent)]
pub struct Flags: u8 {
const BIT = 1 << 0;
const MASK = 0xF << 1;
}
}
let want = Flags::from_bits_retain(3);
let json = serde_json::to_string(&want).unwrap();
let have: Flags = serde_json::from_str(&json).unwrap();
assert_eq!(have, want);
} Output:
Once I added |
I've started work on improving the release notes: https://github.com/bitflags/bitflags/releases/tag/2.0.0 I'll keep working on them to better call out sources of breakage and mitigations for them. Thanks for all the help so far everyone! @mxk Hmm, that looks like a bug to me. You should get the value |
@KodrAus thanks, that is already looking much better! A few nits:
|
Thanks @MarijnS95 👍 I've made a few more changes to the release notes based on your feedback. I think the ship has probably sailed on |
Sure, it would be another breaking change, perhaps it can be queued up for Regardless, Changes look good otherwise, many thanks @KodrAus! |
In trying to update a crate to 2.0.2, |
@molpopgen Hmm, we shouldn't have dropped any bitflags::bitflags! {
pub struct Flags: u8 {
const A = 0b0000_0001;
const B = 0b0000_0010;
const C = 0b0000_0100;
}
} changing only the underlying
Which functions are you seeing reported as no longer |
I am getting this (cut for brevity): The very strange thing is that docs.rs does indeed show those as const. But my local build of the docs shows them as not const: EDIT: I should have been more clear: docs.rs shows these fns as const in the bitflags 2.0.x docs. Building my own docs for my crate locally shows that they are not const with 2.x. But the docs for my crate on docs.rs are const using bitflags 1.3.x, and that is what is triggering semver-checks to fail. |
I've created a new issue for this odd behavior, but I think the changelog is greatly improved over its original wording so will go ahead and close this one. Any more suggestions for improvements can be given in new issues. Thanks for all the input! |
Context
I maintain a few crates that provide
bitflags
types in their public API, while the crates themselves don't really use these types heavily. As such I won't notice changed/missing APIs until these crates are published and used out in the wild.Most importantly, besides listing commit/PR titles in a changelog, it should serve as a human readable "migration guide" explaining the outcome or expected action from consumer crates. Right now this appears to be lacking, forcing consumers to either blindly whack some
derives
on types or features on the crate dependency, or search through this repository and PRs to understand whether that's the right/intended thing to do in the first place.Cannot derive
serde
onbitflags
anymoreSome digging seems points to
bitflags
now being a newtype wrapper around an internal implementation, which requires theserde
feature (disabled by default inserde: enable no-std support by @nim65s in https://github.com/bitflags/bitflags/pull/296
) to be enabled to get this propagated. The entire effect of this newtype wrapper should have had a honorable mention in the changelog 🙂EDIT: Even worse, #282 makes the following mention:
Again, such kind of breakage is exactly the thing to describe in the changelog 😅
Changes to
serde
serialization2.0.0-rc.2
has a big warning that the serialization format changed: https://github.com/bitflags/bitflags/blob/main/CHANGELOG.md#200-rc2, yet this warning was not propagated to the main 2.0.0 changelog: https://github.com/bitflags/bitflags/releases/tag/2.0.0Debug
,Copy
,Clone
,Hash
,(Partial)Eq
etc not automatically derived anymoreAlso appears to be explicitly asked in the issues and happening as a result of the newtype wrapper, but nothing explaining this in the changelog. Would be helpful if it contains a full list of all the trait implementations that have been "removed" going from
1.3.2
to2.0.0
.Regardless of all that, thanks for making all these changes and improvements! I'm sure they'll get appreciated, as long as such breaking effects get the attention they deserve in the changelog.
The text was updated successfully, but these errors were encountered: