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

Move TestExt back into the main package #236

Closed
wants to merge 2 commits into from

Conversation

nhz2
Copy link
Member

@nhz2 nhz2 commented Jul 28, 2024

This reverts #152 and is an alternative to #235

The current strategy of using an extension for testing utilities is causing some issues: Ref #223 #234

This PR moves the functions defined in the extension back into the main package, with slight modifications to avoid depending on Test or Random. The functions aren't documented, so I don't think slightly changing their behavior is breaking.

@nhz2 nhz2 marked this pull request as ready for review July 28, 2024 22:03
@nhz2 nhz2 requested review from mkitti and jakobnissen July 28, 2024 22:03
@mkitti
Copy link
Member

mkitti commented Jul 28, 2024

Is this the right move? Moving this functionality into a subdir package sounds like a better idea.

@nhz2
Copy link
Member Author

nhz2 commented Jul 28, 2024

I copied the functionality into a subdir package in #235
It adds some extra complexity to the CI, and the subdir package needs to be registered before it can be used, but in the long run #235 is probably a more flexible solution.

@nhz2 nhz2 marked this pull request as draft July 29, 2024 00:52
@nhz2 nhz2 closed this Jul 29, 2024
@nhz2 nhz2 deleted the nz/builtin-tests-for-codec-packages branch July 29, 2024 00:56
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