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

chore!: merge fuels-types and fuels-core #956

Merged
merged 16 commits into from
May 31, 2023
Merged

Conversation

hal3e
Copy link
Contributor

@hal3e hal3e commented May 6, 2023

closes: #945

fuels-types is merged into fuels-core. This is the new structure:

fuels-core
├── Cargo.toml
└── src
    ├── codec
    │   ├── abi_decoder.rs
    │   ├── abi_encoder.rs
    │   └── function_selector.rs
    ├── codec.rs
    ├── lib.rs
    ├── traits
    │   ├── parameterize.rs
    │   └── tokenizable.rs
    ├── traits.rs
    ├── types
    │   ├── bech32.rs
    │   ├── core
    │   │   ├── bits.rs
    │   │   ├── bytes.rs
    │   │   ├── native.rs
    │   │   ├── raw_slice.rs
    │   │   └── sized_ascii_string.rs
    │   ├── core.rs
    │   ├── enum_variants.rs
    │   ├── errors.rs
    │   ├── param_types.rs
    │   ├── transaction_builders.rs
    │   ├── unresolved_bytes.rs
    │   ├── wrappers
    │   │   ├── block.rs
    │   │   ├── chain_info.rs
    │   │   ├── coin.rs
    │   │   ├── input.rs
    │   │   ├── message_proof.rs
    │   │   ├── message.rs
    │   │   ├── node_info.rs
    │   │   ├── resource.rs
    │   │   ├── transaction_response.rs
    │   │   └── transaction.rs
    │   └── wrappers.rs
    ├── types.rs
    ├── utils
    │   ├── constants.rs
    │   └── offsets.rs
    └── utils.rs
  • modules utils and types::core and types::wrappers are not exposed. They are used for code organization.
  • traits are moved to fuels-core - breaking!
  • codec is now used to get to ABIEncoder, ABIDecoder, try_from_bytes and resolve_fn_selector - breaking!
  • exposed fuels_core::types only through fuels::types

Checklist

  • I have linked to any relevant issues.
  • I have updated the documentation.
  • I have added necessary labels.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.

@hal3e hal3e added the tech-debt Improves code quality or safety label May 6, 2023
@hal3e hal3e requested a review from a team May 6, 2023 13:05
@hal3e hal3e self-assigned this May 6, 2023
@hal3e hal3e changed the title chore: merge fuels-types and fuels-core chore!: merge fuels-types and fuels-core May 6, 2023
packages/fuels-core/src/types.rs Outdated Show resolved Hide resolved
packages/fuels-test-helpers/Cargo.toml Outdated Show resolved Hide resolved
packages/fuels-core/src/codec.rs Outdated Show resolved Hide resolved
packages/fuels-core/src/utils.rs Outdated Show resolved Hide resolved
packages/fuels-core/src/utils.rs Outdated Show resolved Hide resolved
@iqdecay
Copy link
Contributor

iqdecay commented May 11, 2023

Haven't reviewed yet, but I suggest we hold off on this PR until #950 is merged because it's more pressing than this tech debt. I should push the corrections to the PR today.

Copy link
Contributor

@segfault-magnet segfault-magnet left a comment

Choose a reason for hiding this comment

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

Reviewed, seems good.

Waiting for the fuels types reexports to remove redundancy in the import path.

IIRC we agreed that was to be done here. If not ping me and I'll approve.

Copy link
Contributor

@segfault-magnet segfault-magnet 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.

I'd give up on the idea of shortening the reexport path since even the std does it like we do (e.g. std::result::Result)

Copy link
Member

@Salka1988 Salka1988 left a comment

Choose a reason for hiding this comment

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

👍

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.

nice reorg, feels much cleaner now 🧹

Please make sure to include precise instructions on how to migrate the exports for the user, and include it in a BREAKING CHANGE footer!

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.

Wonderful — ship it!

@hal3e hal3e merged commit 002bc8f into master May 31, 2023
31 checks passed
@hal3e hal3e deleted the hal3e/merge-fuels-core-types branch May 31, 2023 14:18
iqdecay pushed a commit that referenced this pull request Jun 2, 2023
closes: #945
 
- modules `utils` and `types::core` and `types::wrappers` are not
exposed. They are used for code organization.
- `traits` are moved to `fuels-core` - breaking!
- `codec` is now used to get to `ABIEncoder`, `ABIDecoder`,
`try_from_bytes` and `resolve_fn_selector` - breaking!
- exposed `fuels_core::types` only through `fuels::types`
MujkicA pushed a commit that referenced this pull request Jun 5, 2023
closes: #945

- modules `utils` and `types::core` and `types::wrappers` are not
exposed. They are used for code organization.
- `traits` are moved to `fuels-core` - breaking!
- `codec` is now used to get to `ABIEncoder`, `ABIDecoder`,
`try_from_bytes` and `resolve_fn_selector` - breaking!
- exposed `fuels_core::types` only through `fuels::types`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tech-debt Improves code quality or safety
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Merging fuels-types and fuels-core
6 participants