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

refactor: Cleanup test import/exports #551

Merged
merged 2 commits into from
Feb 12, 2024
Merged

refactor: Cleanup test import/exports #551

merged 2 commits into from
Feb 12, 2024

Conversation

pxrl
Copy link
Contributor

@pxrl pxrl commented Feb 12, 2024

Primarily, avoid re-exporting the entirety of contracts-v2 from multiple places in the test directory. This gives better control over what is imported, and where from. Additionally, re-organise so that test/constants only exports actual constants and not utility functions, then relocate the functions to test/utils/utils.ts.

Primarily, avoid re-exporting the entirety of contracts-v2 from multiple
places in the test directory. Additionally, re-organise so that
test/constants only exports actual constants and not utility functions,
then relocate the functions to test/utils/utils.ts.
Copy link
Member

@nicholaspai nicholaspai left a comment

Choose a reason for hiding this comment

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

Why are we increasing the dependency on contracts-v2?

@pxrl
Copy link
Contributor Author

pxrl commented Feb 12, 2024

Why are we increasing the dependency on contracts-v2?

I'd say we're just making the dependencies explicit, rather than implicit. One of the drivers for this PR was me being unhappy with users importing from test/constants implicitly picking up dependencies from contracts-v2 without realising it (see screenshot below). This PR fixes that by forcing the consumer to import contractsV2Utils from test/utils/utils.ts, or importing one of the types or functions that are explicitly exported via test/utils/utils.ts or constants from test/constants.ts.

In future we should probably refine this further by forcing the user to just import contractsV2Utils.

diff

@pxrl pxrl merged commit a201093 into master Feb 12, 2024
3 checks passed
@pxrl pxrl deleted the pxrl/testCleanup branch February 12, 2024 13:24
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