Skip to content

io: Update defmt to 1.0, rename feature from "defmt_03" to "defmt" #673

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

Merged
merged 1 commit into from
Aug 7, 2025

Conversation

chrysn
Copy link
Contributor

@chrysn chrysn commented Aug 3, 2025

We could do the defmt-1.0 update a bit more compatibly by just going to 0.3.100 (and possibly keeping the defmt_03 feature as a deprecated feature name), but given the discussion in #566 (comment), it may make sense to spool this for a single breaking >>0.6 bump, and use that to get rid of anything that'd be viewed as legacy in a 1.0 version.

One alternative to the hard cut would be to to keep a defmt_03 feature that merely pulls in 0.3.100, as a deprecated feature. If >>0.6 becomes a 0.7 that does the semver trick to 1.0, that feature could be removed as part of the 1.0 transition.

An extra deprecated defmt-03 feature is left in place mainly for embedded-io-async, which optionally pulls it in through the path-or-version dependency.

@chrysn chrysn requested a review from a team as a code owner August 3, 2025 08:29
Copy link
Contributor

@rursprung rursprung 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 taking care of this!

Copy link
Contributor

@rursprung rursprung left a comment

Choose a reason for hiding this comment

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

i'd just mention the major version and skip minor&patch - there are no specific requirements here (that'd only make sense if you'd e.g. depend on 1.2.3 and really need something which was introduced only with that version; but 1 = 1.0 = 1.0.0).

(but that's just my opinion; i'm no maintainer)

otherwise this LGTM - thanks!

@chrysn
Copy link
Contributor Author

chrysn commented Aug 7, 2025

Thanks for reviewing; your comments all being applied, I've taken the liberty to squash and rebase them into a sensible history for the maintainer team's review.

Copy link
Contributor

@rursprung rursprung left a comment

Choose a reason for hiding this comment

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

perfect, thanks!

in the nit-picking territory: just note that with the two commits i'd usually expect that i could only build the first without the second but that wouldn't be the case here as then e-io-async would depend on a non-existent feature in e-io. so you might want to squash the two together completely or already use the defmt feature flag in the e-io dependency in e-io-async in the first commit.

@Dirbaio Dirbaio added this pull request to the merge queue Aug 7, 2025
Merged via the queue into rust-embedded:master with commit de0d966 Aug 7, 2025
7 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.

3 participants