-
Notifications
You must be signed in to change notification settings - Fork 189
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
refactor: combine name and namespace to resource selector in World methods #1208
Conversation
🦋 Changeset detectedLatest commit: 28a0268 The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
fe0fc1d
to
eaf16cf
Compare
0b8a66d
to
7ff27a1
Compare
eaf16cf
to
39992b2
Compare
7ff27a1
to
4d918cd
Compare
3658f6d
to
3796fec
Compare
4d918cd
to
4a52c4a
Compare
3796fec
to
df01ee1
Compare
4a52c4a
to
c49c11e
Compare
df01ee1
to
d139d9a
Compare
c49c11e
to
8a2d0d1
Compare
d139d9a
to
00f9b9f
Compare
8a2d0d1
to
0ca3474
Compare
00f9b9f
to
f86e487
Compare
0ca3474
to
2dc150b
Compare
f86e487
to
79b0e0d
Compare
b307f55
to
57beee8
Compare
@@ -1,5 +1,6 @@ | |||
import { Hex, stringToHex, concatHex } from "viem"; | |||
|
|||
// TODO: rename to `resourceIdToHex` or `resourceSelectorToHex` since it can be used with other resources than tables |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but this is specific to tables (namespace+name), not all resources follow that same naming scheme, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see now that we've sprinkled this throughout our deploy process etc.
Maybe worth renaming this but still exposing a table ID variant/wrapper that does ~the same thing for clarity when working with tables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, just some nits
virtual/override look inconsistent in World (u're not using virtual in some places, and using override for interfaces)
remove separate function selectors for namespace/name in World and instead align with Store interfaces on only using bytes32 resourceSelector
side effect: saving some byte code in the world contract: