Allow bitflags v1 and v2 to coexist #894
Conversation
|
Unfortunately this doesn't work because Library A might use defmt to describe some data type and expect the bitflags v1 API whilst Library B might use defmt but expect the bitflags v2 API (if we add it). You could have a feature flag for bitflags v2 so that we keep the dependency list as short as possible, but it has to be additional and exported as a different macro with a different name. |
manushT
force-pushed
the
allow-bitflags-v1-and-v2-to-coexist
branch
4 times, most recently
from
705a4d2 to
aabac90
Compare
|
The CHANGELOG was reformatted. Can you rebase? |
manushT
force-pushed
the
allow-bitflags-v1-and-v2-to-coexist
branch
2 times, most recently
from
0ffa167 to
5f6ef8a
Compare
|
The format check failed, I'm afraid. |
CHANGELOG.md
Outdated
| @@ -69,6 +69,7 @@ We have several packages which live in this repository. Changes are tracked sepa | |||
| * [#845] `decoder`: fix println!() records being printed with formatting | |||
| * [#843] `defmt`: Sort IDs of log msgs by severity to allow runtime filtering by severity | |||
| * [#822] `CI`: Run `cargo semver-checks` on every PR | |||
| * [#877]:`defmt`: Allow `bitflags` v1 and v2 to coexist | |||
There was a problem hiding this comment.
- This change was made to Defmt-next because defmt-0.3.9 has already been released so it needs to be in the section above.
- This should refer to the PR, which is Allow bitflags v1 and v2 to coexist #894.
- You will need to add a URL for
[#894]down at the bottom of the file for the link to be valid.
manushT
force-pushed
the
allow-bitflags-v1-and-v2-to-coexist
branch
2 times, most recently
from
b923d69 to
b27ffe3
Compare
CHANGELOG.md
Outdated
| @@ -27,6 +27,7 @@ We have several packages which live in this repository. Changes are tracked sepa | |||
| > A highly efficient logging framework that targets resource-constrained devices, like microcontrollers | |||
|
|
|||
| [defmt-next]: https://github.com/knurling-rs/defmt/compare/defmt-v0.3.9...main | |||
| [defmt-next]: https://github.com/knurling-rs/defmt/pull/894 | |||
There was a problem hiding this comment.
We already have a markdown link for [defmt-next] which was correct. You can't add another.
The change you've made needs to be listed under ## [defmt-next] heading.
manushT
force-pushed
the
allow-bitflags-v1-and-v2-to-coexist
branch
from
b27ffe3 to
c3cca96
Compare
manushT
force-pushed
the
allow-bitflags-v1-and-v2-to-coexist
branch
2 times, most recently
from
bf53871 to
58cab23
Compare
manushT
force-pushed
the
allow-bitflags-v1-and-v2-to-coexist
branch
from
58cab23 to
5a22752
Compare
| @@ -361,11 +361,21 @@ pub use defmt_macros::timestamp; | |||
| /// const ABC = Self::A.bits | Self::B.bits | Self::C.bits; | |||
| /// } | |||
| /// } | |||
| /// | |||
| /// #[cfg(feature = "bitflagsv2")] | |||
There was a problem hiding this comment.
I think we need some more documentation here to explain why we have two bitflags macros now and how users should pick between them. And actually, currently bitflagsv2 is completely undocumented because the new docs were added to the block for bitflags. You can check with cargo doc -p defmt --features=bitflagsv2 --open.
macros/src/items/bitflags.rs
Outdated
| let bits_access = if is_v2 { | ||
| quote! { bits() } | ||
| } else { | ||
| quote! { bits} |
There was a problem hiding this comment.
| quote! { bits} | |
| quote! { bits } |
|
@manushT - we still have an outstanding comment on this one. |
Urhengulas
added
the
pr waits on: author
The patch breaks cargo's additive features
Fixes #877.