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>
fitzgen commented on issue #7078:
Mind adding a
hash_key
toTable
for consistency, even though it's not used here? Also mind adding some API tests thathash_key
is the same value if, for example, a global is extracted twice?Hm as I say this
Func
doesn't implementhash_key
either, so the "inconsistency" of tables not implementing it may be ok. CouldFunc
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.
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.
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.
Last updated: Dec 23 2024 at 12:05 UTC