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

easy(move-build): assorted clean ups #21157

Merged
merged 3 commits into from
Feb 13, 2025
Merged

Conversation

amnn
Copy link
Contributor

@amnn amnn commented Feb 10, 2025

Description

A couple of small clean-ups in Move-related code that I noticed while working on other things:

  • Removing an unnecessary lifetime parameter on SuiAdapter impl block.
  • Removing a duplicate function on CompiledPackage.
  • Richer error message when Modules detects a duplicate Module by ID.

Test plan

CI

Stack


Release notes

Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.

For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.

  • Protocol:
  • Nodes (Validators and Full nodes):
  • gRPC:
  • JSON-RPC:
  • GraphQL:
  • CLI:
  • Rust SDK:

@amnn amnn self-assigned this Feb 10, 2025
Copy link

vercel bot commented Feb 10, 2025

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

Name Status Preview Comments Updated (UTC)
sui-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 13, 2025 2:54pm
2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Feb 13, 2025 2:54pm
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Feb 13, 2025 2:54pm

Copy link
Contributor

@mdgeorge4153 mdgeorge4153 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@amnn amnn force-pushed the amnn/clean-move-build branch from d596e6c to eba4d34 Compare February 12, 2025 15:48
@amnn amnn temporarily deployed to sui-typescript-aws-kms-test-env February 12, 2025 15:48 — with GitHub Actions Inactive
@amnn amnn force-pushed the amnn/clean-move-build branch from eba4d34 to 96492d2 Compare February 12, 2025 22:38
@amnn amnn temporarily deployed to sui-typescript-aws-kms-test-env February 12, 2025 22:39 — with GitHub Actions Inactive
@amnn amnn changed the base branch from amnn/rpc-mv-fun to amnn/rpc-owned-objs February 13, 2025 11:47
@amnn amnn mentioned this pull request Feb 13, 2025
7 tasks
amnn added 3 commits February 13, 2025 14:06
## Description

Don't need the lifetime parameter on `impl SuiTestAdapter`.

## Test plan

CI
## Description

`CompiledPackage::published_dependency_ids` is a duplicate of
`CompiledPackage::get_dependency_storage_package_ids`. Get rid of it and
replace its only call-site.

## Test plan

CI
## Description
If we find a duplicate module when creating a `Modules` map, include the
module's ID in the output.

## Test plan
Hit this error when debugging something else.
@amnn amnn force-pushed the amnn/rpc-owned-objs branch from 4cae684 to 6ffec5a Compare February 13, 2025 14:35
@amnn amnn force-pushed the amnn/clean-move-build branch from 96492d2 to 7c4e5c1 Compare February 13, 2025 14:36
@amnn amnn temporarily deployed to sui-typescript-aws-kms-test-env February 13, 2025 14:36 — with GitHub Actions Inactive
@amnn amnn merged commit 7c4e5c1 into amnn/rpc-owned-objs Feb 13, 2025
44 checks passed
@amnn amnn deleted the amnn/clean-move-build branch February 13, 2025 16:19
@amnn amnn temporarily deployed to sui-typescript-aws-kms-test-env February 13, 2025 16:19 — with GitHub Actions Inactive
@amnn amnn temporarily deployed to sui-typescript-aws-kms-test-env February 13, 2025 16:19 — with GitHub Actions Inactive
@amnn amnn temporarily deployed to sui-typescript-aws-kms-test-env February 13, 2025 16:20 — with GitHub Actions Inactive
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.

2 participants