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

Merge std and core libraries #6729

Merged
merged 107 commits into from
Mar 12, 2025
Merged

Merge std and core libraries #6729

merged 107 commits into from
Mar 12, 2025

Conversation

SwayStar123
Copy link
Collaborator

@SwayStar123 SwayStar123 commented Nov 18, 2024

Description

Merges the two libraries. They were initially separate to separate the core logic and fuel vm specific functionality, but that separation is no longer maintained so having a merged library is better.

Closes #6708

Checklist

  • I have linked to any relevant issues.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation where relevant (API docs, the reference, and the Sway book).
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added (or requested a maintainer to add) the necessary Breaking* or New Feature labels where relevant.
  • 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.

@SwayStar123 SwayStar123 added compiler General compiler. Should eventually become more specific as the issue is triaged big this task is hard and will take a while lib: core Core library forc-pkg Everything related to the `forc-pkg` crate. labels Mar 10, 2025
Copy link
Contributor

@jjcnn jjcnn left a comment

Choose a reason for hiding this comment

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

Truly a Herculian effort on this one!

I've added a few comments. There shouldn't be much work in them, so there should be time to do them before the release.

The ones about using the reduced std libs can be left for a separate issue if needed.

Copy link
Contributor

@jjcnn jjcnn left a comment

Choose a reason for hiding this comment

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

Changing my review status to approved.

The one change that was definitely needed was comments in abi_encoding.rs, which have been added.

My remaining review comments are nits.

Copy link
Member

@bitzoic bitzoic left a comment

Choose a reason for hiding this comment

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

I would have liked to see more edge cases covered(zero and max values) in the inline tests but these can be added afterward

Copy link
Member

@kayagokalp kayagokalp left a comment

Choose a reason for hiding this comment

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

Tooling side of the things looks good to me. We used to have that if core is not added add it by default and std was opt-out. Now that we remove core we seem to remove the auto core addition thingy as well. Just writing to confirm that this is the desired behaviour.

@ironcev ironcev merged commit a5d9d28 into master Mar 12, 2025
44 checks passed
@ironcev ironcev deleted the swaystar123/merge-core-std branch March 12, 2025 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
big this task is hard and will take a while breaking May cause existing user code to break. Requires a minor or major release. compiler General compiler. Should eventually become more specific as the issue is triaged forc-pkg Everything related to the `forc-pkg` crate. lib: core Core library lib: std Standard library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Combine std-lib and core
9 participants