Skip to content
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

move: bump Move.lock files in framework packages #15194

Merged
merged 1 commit into from
Dec 9, 2023

Conversation

rvantonder
Copy link
Contributor

@rvantonder rvantonder commented Dec 4, 2023

Description

Stacked on #15166.

Ran sui move build in framework-packages.

Test Plan

N/A


If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.

Type of Change (Check all that apply)

  • protocol change
  • user-visible impact
  • breaking change for a client SDKs
  • breaking change for FNs (FN binary must upgrade)
  • breaking change for validators or node operators (must upgrade binaries)
  • breaking change for on-chain data layout
  • necessitate either a data wipe or data migration

Release notes

Copy link

vercel bot commented Dec 4, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
mysten-ui ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 8, 2023 11:36pm
sui-core ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 8, 2023 11:36pm
sui-typescript-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 8, 2023 11:36pm
3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
explorer ⬜️ Ignored (Inspect) Visit Preview Dec 8, 2023 11:36pm
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Dec 8, 2023 11:36pm
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Dec 8, 2023 11:36pm

@rvantonder rvantonder marked this pull request as ready for review December 4, 2023 20:54
@@ -21,6 +21,7 @@ Move.lock
!crates/sui-framework/packages/move-stdlib/Move.lock
!crates/sui-framework/packages/sui-framework/Move.lock
!crates/sui-framework/packages/sui-system/Move.lock
!crates/sui-framework/packages/deepbook/Move.lock
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we track other packages in sui-framework so deepbook should be included as well

@rvantonder rvantonder requested review from amnn and a team December 4, 2023 20:55
Copy link
Member

@amnn amnn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change seems fine to me, but does this mean that moving forward we'll need to make this part of the release process or something? (i.e. how does toolchain versioning work for system packages?)

@rvantonder
Copy link
Contributor Author

i.e. how does toolchain versioning work for system packages?

Good question, I'm running into some of this with the implementation too. Let me fold that into a larger discussion that I'll raise in slack. My overall take is: if we verify against on-chain system packages, it would imply we're using the latest version (and we can bake that into our versioning implementation with or without checking Move.lock).

Right now, if #15166 goes live, system package builds will modify Move.lock, so it makes sense to check in, which is the main reason behind this change. At the same time, we haven't always checked in changes digest changes either, so we could punt / ignore this Move.lock depending on how we treat it in toolchain versioning. In essence, writing this to the lock doesn't imply we need to use or make this part of the release process yet, until we discuss more. My sense is it that updates will likely just count as good hygiene.

@amnn
Copy link
Member

amnn commented Dec 6, 2023

Got it, thanks for the context! If we do go down the route of checking in these changes, we should probably also have a CI check (or update the existing changed_files.sh one) to make sure that a necessary lock file update was checked in.

Base automatically changed from rvt/write-lock-file-3 to main December 8, 2023 23:31
@rvantonder rvantonder merged commit 2ff0ec1 into main Dec 9, 2023
39 checks passed
@rvantonder rvantonder deleted the rvt/update-framework-locks branch December 9, 2023 01:48
gdanezis pushed a commit that referenced this pull request Dec 15, 2023
## Description 

As in title

## Test Plan 

N/A

---
If your changes are not user-facing and not a breaking change, you can
skip the following section. Otherwise, please indicate what changed, and
then add to the Release Notes section as highlighted during the release
process.

### Type of Change (Check all that apply)

- [ ] protocol change
- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants