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: export test and cli utilities in fuels umbrella package #2637

Merged
merged 20 commits into from
Jul 2, 2024

Conversation

nedsalk
Copy link
Contributor

@nedsalk nedsalk commented Jun 27, 2024

I exported the utils via the fuels umbrella package and changed imports to import from fuels accordingly. The changes in this PR are exclusively tied to that and running pnpm lint:fix afterwards.

@nedsalk nedsalk added the chore Issue is a chore label Jun 27, 2024
@nedsalk nedsalk self-assigned this Jun 27, 2024
@nedsalk nedsalk marked this pull request as ready for review June 27, 2024 13:11
danielbate
danielbate previously approved these changes Jun 28, 2024
@arboleya arboleya added this to the 0.x mainnet milestone Jun 28, 2024
Copy link
Contributor

@petertonysmith94 petertonysmith94 left a comment

Choose a reason for hiding this comment

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

I'll approve

I think we should test the next tagged CLI after it's merged into release :)

arboleya
arboleya previously approved these changes Jun 29, 2024
@nedsalk nedsalk dismissed stale reviews from arboleya, petertonysmith94, and danielbate via 807e4d3 July 1, 2024 07:44
@nedsalk nedsalk requested a review from arboleya July 1, 2024 07:44
@nedsalk nedsalk enabled auto-merge (squash) July 1, 2024 08:36
Torres-ssf
Torres-ssf previously approved these changes Jul 1, 2024
Copy link
Contributor

@maschad maschad left a comment

Choose a reason for hiding this comment

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

This looks fine, but a naive search in my code editor shows up 134 files, could we be missing some changes here? Or am I miscalculating?

Screenshot 2024-07-01 at 11 11 14 AM

@nedsalk
Copy link
Contributor Author

nedsalk commented Jul 1, 2024

@maschad you searched for /test-utils which is showing results for fuels/test-utils, @fuel-ts/utils/test-utls, @fuel-ts/account/test-utils, etc. This consolidates it all to use import from fuels/test-utils.

Copy link
Contributor

@maschad maschad left a comment

Choose a reason for hiding this comment

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

I checked out onto this branch there are still a number of files that import from:

  • @fuel-ts/errors/test-utils
  • @fuel-ts/account/test-utils
  • @fuel-ts/utils/test-utils

There's also some outdated docs / snippets such as:

@maschad
Copy link
Contributor

maschad commented Jul 1, 2024

@maschad you searched for /test-utils which is showing results for fuels/test-utils, @fuel-ts/utils/test-utls, @fuel-ts/account/test-utils, etc. This consolidates it all to use import from fuels/test-utils.

Thanks for the clarification, if you update the search to be explicit for @fuel-ts/errors/test-utils
@fuel-ts/account/test-utils and @fuel-ts/utils/test-utils you will see quite a few files. I've elaborated more here

Copy link
Member

@danielbate danielbate left a comment

Choose a reason for hiding this comment

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

@maschad we should still be importing from the mono packages themselves rather than the umbrella inside the packages though shouldn't we? Filter your search to check only where we are installing fuels i.e. packages/fuel-gauge and docs-snippets. But you're correct around outdated docs.

@nedsalk nedsalk dismissed stale reviews from Torres-ssf and petertonysmith94 via 5131778 July 2, 2024 12:24
@nedsalk
Copy link
Contributor Author

nedsalk commented Jul 2, 2024

@danielbate is right in #2637 (review).

As for the snippets and some leftovers that I didn't fix, I fixed them in f980df5 and 5131778.

Copy link
Contributor

@maschad maschad left a comment

Choose a reason for hiding this comment

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

Thanks for the clarification @danielbate

Good job @nedsalk

Copy link
Contributor

github-actions bot commented Jul 2, 2024

Coverage Report:

Lines Branches Functions Statements
79.54%(+0%) 71.37%(+0%) 76.91%(+0%) 79.62%(+0%)
Changed Files:

Coverage values did not change👌.

@nedsalk nedsalk merged commit 041805f into master Jul 2, 2024
19 checks passed
@nedsalk nedsalk deleted the ns/chore/export-utilities-in-umbrella-package branch July 2, 2024 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Issue is a chore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Export [test] utilities in umbrella package
6 participants