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

Add fuels-types minimal crate for declaring serializable ABI types. Resolves the cycle between fuels-rs and sway repos. #219

Merged
merged 4 commits into from
Apr 21, 2022

Conversation

mitchmindtree
Copy link
Contributor

This moves the JsonABI type along with the associated Function and Property types from sway-types into a new minimal fuels-types crate.

By moving these from sway-types upstream to the fuels-rs repo, we can resolve the remainder of the cyclic dependency between the sway and fuels-rs repositories.

I think also semantically it makes a little more sense for the Rust representation of these ABI-specific types to be declared under the Rust SDK, and for Sway to depend on these rather than vice versa.

The fuels-core crate now depends on these types via the new fuels-types crate. The unused sway-utils dependency has been removed.

As an immediate follow-up to fuels-types landing and getting published to crates.io, we should remove these types from the sway-types crate and instead depend on fuels-types as necessary throughout the sway repo.

This is one of the blockers for FuelLabs/sway#1321.

By moving these from `sway-types` upstream to the `fuels-rs` repo, we
can resolve the remainder of the cyclic dependency between the `sway`
and `fuels-rs` repositories.

I think semantically it also makes more sense for these VM-specific
types to be declared upstream, and for Sway to depend on these rather
than vice versa.
Also removes the unused `sway-types` dependency from the
`fuels-contract` crate.
@mitchmindtree mitchmindtree added enhancement New feature or request tech-debt Improves code quality or safety labels Apr 20, 2022
@mitchmindtree mitchmindtree self-assigned this Apr 20, 2022
@mitchmindtree mitchmindtree changed the title Add fuels-types minimal crate for declaring serializable ABI types. Resolves the cycle between fuels-rs and sway repos. Add fuels-types minimal crate for declaring serializable ABI types. Resolves the cycle between fuels-rs and sway repos. Apr 20, 2022
JoshuaBatty
JoshuaBatty previously approved these changes Apr 20, 2022
Copy link
Member

@JoshuaBatty JoshuaBatty left a comment

Choose a reason for hiding this comment

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

utACK. Nice one

Copy link
Contributor

@adlerjohn adlerjohn left a comment

Choose a reason for hiding this comment

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

Nice, looks like this finally breaks the circular dependency! I'll defer to @digorithm

Copy link
Contributor

@iqdecay iqdecay left a comment

Choose a reason for hiding this comment

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

GG! There will be a corresponding PR for Sway if I understand correctly? To make it import from this new crate?

Copy link
Member

@digorithm digorithm left a comment

Choose a reason for hiding this comment

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

Great stuff, thanks for doing this @mitchmindtree!

@mitchmindtree mitchmindtree merged commit 5bd29e7 into master Apr 21, 2022
@mitchmindtree mitchmindtree deleted the mitchmindtree/fuels-types branch April 21, 2022 00:15
mitchmindtree added a commit to FuelLabs/sway that referenced this pull request Apr 21, 2022
mitchmindtree added a commit to FuelLabs/sway that referenced this pull request Apr 21, 2022
mitchmindtree added a commit to FuelLabs/sway that referenced this pull request May 2, 2022
mitchmindtree added a commit to FuelLabs/sway that referenced this pull request May 7, 2022
mitchmindtree added a commit to FuelLabs/sway that referenced this pull request May 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request tech-debt Improves code quality or safety
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants