Stream: git-wasmtime

Topic: wasmtime / issue #7078 Make `wasmtime::WasmCoreDump` seri...

Wasmtime GitHub notifications bot (Sep 22 2023 at 18:44):

github-actions[bot] commented on issue #7078:

Subscribe to Label Action

cc @peterhuene

<details>
This issue or pull request has been labeled: "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

• peterhuene: wasmtime:api

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Sep 25 2023 at 17:08):

fitzgen commented on issue #7078:

Mind adding a hash_key to Table for consistency, even though it's not used here? Also mind adding some API tests that hash_key is the same value if, for example, a global is extracted twice?

Hm as I say this Func doesn't implement hash_key either, so the "inconsistency" of tables not implementing it may be ok. Could Func implement it too though?

Opted not to do this in this PR. If you feel strongly I can do it in a follow up.

But nothing would actually use these new methods, so we would have to #[allow(unused)] which feels pretty bleh just for aesthetic consistency. They are super easy methods to add, should we ever need them. Just a couple line copy paste.

Wasmtime GitHub notifications bot (Sep 25 2023 at 17:11):

alexcrichton commented on issue #7078:

It's ok to do in a follow-up yeah, but the use I was envisioning would be tests using the methods and that's it.

They are super easy methods to add, should we ever need them. Just a couple line copy paste.

FWIW in my experience while I don't disagree with this it's typically not you who would end up needing this but a contributor down the line and the effort for a contributor to add the methods is often significantly higher than us who regularly maintain Wasmtime. This line of thinking applies to all small changes, though, so it's not necessarily great in that respect.

Wasmtime GitHub notifications bot (Sep 25 2023 at 19:25):

fitzgen commented on issue #7078:

It's ok to do in a follow-up yeah, but the use I was envisioning would be tests using the methods and that's it.

https://github.com/bytecodealliance/wasmtime/pull/7086

Last updated: Nov 22 2024 at 16:03 UTC