search

jamesmunns / postcard

A no_std + serde compatible message library for Rust
Apache License 2.0
874 stars 87 forks source link

Postcard 1.0.8 pulls in `embedded-io` with the `alloc` feature #158

Closed robertbastian closed 1 month ago

robertbastian commented 4 months ago

This is because the alloc feature enables embedded-io/alloc, instead of embedded-io?/alloc.

This is fixed by #123, which just needs to be released.

robertbastian commented 2 months ago

Bump

jamesmunns commented 2 months ago

Hi @robertbastian, please don't bump issues.

I've not addressed this because I haven't had time to figure out what the plan is for https://github.com/jamesmunns/postcard/issues/128, and the refactoring necessary to avoid pulling in external deps as part of the core crate.

I'm currently busy with other work, and will get to this when I am able.

robertbastian commented 2 months ago

Well this has gone unanswered for almost two months, so thanks for the response.

Is the repo currently not semver compatible with the latest crates.io release?

robertbastian commented 2 months ago

As far as I can tell, #91 has been released (which caused this issue), #123 fixes it, and #123 is careful to add support for the new embedded-io version in a semver-compatible way. If you don't want to release #123, would you accept a PR on top of v1.0.7 to fix this?

jamesmunns commented 2 months ago

Well this has gone unanswered for almost two months, so thanks for the response.

If you are interested in a SLA or maintenance contract with guaranteed response times, please feel free to reach out via email to discuss, I'm more than happy to offer it!

Is the repo currently not semver compatible with the latest crates.io release?

I'm not certain, I started working towards the 2.0 release, but was pulled off on other contract work.

This is the current delta since 1.0.8, the last tagged release: https://github.com/jamesmunns/postcard/compare/v1.0.8...main

If you can validate that the current state of main is semver compatible with 1.0.8 using a tool like cargo-semver-checks, I can go ahead and cut a release. If it is not, I would accept a new 1.0.x branch forked off at some point between the v1.0.8 tag and HEAD, which fixes any semver regressions, and I can base a release off of that branch.

I've not done this as there are other PRs I haven't had time to review that people would like merged, and I would like to avoid maintaining two release trains if at all possible.

robertbastian commented 2 months ago

Ok, main is not semver compatible because the heapless version has been upgraded. However, #123, which is the first commit since the last release and fixes this issue, is semver compatible, and could be released as 1.0.9. Alternatively, this would be the minimal fix: https://github.com/jamesmunns/postcard/compare/main...robertbastian:postcard:fix.

jamesmunns commented 2 months ago

Is the heapless update the only breaking change you've found? Would backing that change out restore the current main branch to be semver compatible?

If so, I'd also be willing to take a revert PR that backs that out on the main branch, which would mean that we wouldn't lose the other nice-to-have PRs merged in the intervening time. I'd say that'd be my preferred option.

Failing that, you'd propose forking off of https://github.com/jamesmunns/postcard/commit/2ddc52bfebfb0a3ecdccb06909b1fb9a8fd70757 instead?

robertbastian commented 2 months ago

Is the heapless update the only breaking change you've found? Would backing that change out restore the current main branch to be semver compatible?

I believe so.

I'd be fine with either of the three options (releasing my commit off 2ddc52b, releasing at 2ddc52bfebfb0a3ecdccb06909b1fb9a8fd70757, or backing out the heapless change and releasing from main).

Manishearth commented 1 month ago

I'm also happy to help make this happen; I can make the backout PR and a release PR, assuming the release workflow is just landing a Cargo.toml bump and releasing.

jamesmunns commented 1 month ago

Yep! Sorry I didn't respond, I am open to any of the proposals Robert or I suggested! Weak preference to "PR to backout on main" just to keep the history linear, as mentioned above.

If you do the backout PR with a clean cargo-semver-checks bill of health, I can merge and do the release as soon as its merged.

Manishearth commented 1 month ago

Cool, I'll do that later today then

Manishearth commented 1 month ago

https://github.com/jamesmunns/postcard/pull/164

jamesmunns commented 1 month ago

CC @robertbastian @Manishearth just released v1.0.9, could you please confirm this works to resolve the issue you saw?

https://github.com/jamesmunns/postcard/releases/tag/v1.0.9

robertbastian commented 1 month ago

Yes, this fixes the issue. Thanks!