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

Remove the direct dependency from sui-core to sui-move-build #20706

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

mdgeorge4153
Copy link
Contributor

@mdgeorge4153 mdgeorge4153 commented Dec 20, 2024

Description

Previously, sui-move-build is linked into sui-core as a normal dependency. The only reason for this is that some of the sui-core testing utilities depend on sui-move-build and some other crates depend on the sui-core testing utilities. This PR cleans this up a bit so that the testing infrastructure that depends on sui-move-build is not re-exported, and therefore sui-move-build only needs to be a dev-dependency of sui-core.

Specifically, I split the sui_core::test_utils code that is only used inside of sui-core off into sui_core::unit_test_utils, and gated this file behind #[cfg(test)]. I then moved the tiny bit of shared testing code that uses sui-move-build into sui_move_build::test_utils.

Similarly, I split off some of sui_core::authority::authority_test_utils into sui_core::authority::auth_unit_test_utils.

Test plan

Since this just involves moving things around, if everything builds and the existing tests pass I will consider it adequately tested.


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):
  • Indexer:
  • JSON-RPC:
  • GraphQL:
  • CLI:
  • Rust SDK:
  • REST API:

Copy link

vercel bot commented Dec 20, 2024

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 Dec 20, 2024 9:31pm
3 Skipped Deployments
Name Status Preview Comments Updated (UTC)
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Dec 20, 2024 9:31pm
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Dec 20, 2024 9:31pm
sui-typescript-docs ⬜️ Ignored (Inspect) Visit Preview Dec 20, 2024 9:31pm

@mdgeorge4153 mdgeorge4153 temporarily deployed to sui-typescript-aws-kms-test-env December 20, 2024 21:28 — with GitHub Actions Inactive
@mdgeorge4153 mdgeorge4153 temporarily deployed to sui-typescript-aws-kms-test-env December 20, 2024 21:29 — with GitHub Actions Inactive
@mdgeorge4153 mdgeorge4153 requested review from amnn and tzakian December 20, 2024 21:34
@mdgeorge4153 mdgeorge4153 marked this pull request as ready for review December 20, 2024 21:40
@mdgeorge4153 mdgeorge4153 temporarily deployed to sui-typescript-aws-kms-test-env December 20, 2024 21:40 — with GitHub Actions Inactive
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.

👍

FWIW, another approach that may have been simpler/more mechanical would have been to pull out the sui-core::test_utils into their own crate which depends on sui-core and sui-move-build and is itself a dev-dependency of all the other crates that were depending on sui-core for this functionality before.

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