Stream: git-wasmtime

Topic: wasmtime / PR #2805 Release 0.26.0.


view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2021 at 17:29):

cfallin opened PR #2805 from release-0.26.0 to main:

This PR bumps versions of all crates (Wasmtime to 0.26.0, Cranelift to 0.73.0) and adds release notes.

Please let me know if I missed any important PRs or got the descriptions wrong on any of the release-note entries!

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2021 at 17:38):

peterhuene submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2021 at 17:38):

peterhuene created PR Review Comment:

Perhaps we should just leave the version number off the assertion so it's not a maintenance issue on version bumps?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2021 at 17:38):

peterhuene submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2021 at 17:48):

cfallin updated PR #2805 from release-0.26.0 to main.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2021 at 17:49):

cfallin submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2021 at 17:49):

cfallin created PR Review Comment:

Good point -- now it's just .starts_with("Module was compiled with incompatible Wasmtime version"). Thanks!

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2021 at 17:50):

bjorn3 submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2021 at 17:50):

bjorn3 created PR Review Comment:

            .starts_with("Module was compiled with an incompatible Wasmtime version")),

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2021 at 17:52):

cfallin submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2021 at 17:52):

cfallin created PR Review Comment:

This is matching an explicit error string that doesn't have the "an"; are you suggesting changing the wording of the error itself? Perhaps that's best as a separate PR?

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2021 at 17:52):

bjorn3 submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2021 at 17:52):

bjorn3 created PR Review Comment:

The maintenance issue could be prevented by overwriting the whole version number instead of just the first part I think. Keeping the version number could still be useful.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2021 at 18:10):

peterhuene submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2021 at 18:10):

peterhuene created PR Review Comment:

I think this only reads incorrectly when truncated and my opinion is that it doesn't need changing, but we can update the error message in another PR if necessary, I think.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2021 at 18:12):

cfallin submitted PR Review.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2021 at 18:12):

cfallin created PR Review Comment:

That gets a little more complex, I think, especially when the version string gets longer (0.100.0)... this is a little too in-the-weeds IMHO for a simple "does the serialization check the version" test; any change in the version should suffice and any error message that starts with "Module was compiled with incompatible Wasmtime version" is certainly the right error that we want to see. If @peterhuene wants to make the test fancier in the future then we can certainly do that but I don't think we should hold the release for that.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 05 2021 at 19:01):

cfallin merged PR #2805.


Last updated: Dec 23 2024 at 13:07 UTC