-
Notifications
You must be signed in to change notification settings - Fork 88
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
feat: add model upgrades page #385
feat: add model upgrades page #385
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Thank you @remybar for the details here, added a small proposal, and I'll merge once we have the PR in dojo merged. 👍
- For composite data structures like `struct`, `enum`, `tuple` and `array` | ||
- they are upgreadable as soon as all their elements are upgreadable, | ||
- existing elements cannot be removed, they can only be modified according to the rules described | ||
on this page. New elements can be freely added. |
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.
on this page. New elements can be freely added. | |
on this page. |
This one is vague since tuple can't have new elements added.
Wdyt about this summary:
struct
: new members can be added or their types modified following the rules below in this page. Renaming a field is not permitted.enum
: new variants can be added, but existing ones can't be renamed. The type of a variant can be changed following the rules below.tuple
: types inside the tuple may be changed following the rules below, however the tuple size can't be changed.array
: the inner type of the array can be changed following the rules below.
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.
To be more consistent, I changed the PR to allow increasing the size of tuples as it has no impact on existing stored model data.
Knowing that, do you still think this sentence has to be more detailed ? Because the following rules are already described and match with all the composite types:
- element name cannot change,
- element type can only change according to the rules.
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.
Not sure if adding tuples is supported by Torii, and I've told @Larkooo during the Torii update that the tuples will not change in size. We may keep a tuple fix in size and only allow its types to grow. Or are you seeing a very important gain to have tuples that can grow?
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.
Thank you @remybar!
- For composite data structures like `struct`, `enum`, `tuple` and `array` | ||
- they are upgreadable as soon as all their elements are upgreadable, | ||
- existing elements cannot be removed, they can only be modified according to the rules described | ||
on this page. New elements can be freely added. |
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.
Not sure if adding tuples is supported by Torii, and I've told @Larkooo during the Torii update that the tuples will not change in size. We may keep a tuple fix in size and only allow its types to grow. Or are you seeing a very important gain to have tuples that can grow?
Related to dojoengine/dojo#2925.