Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Make Test a weak dependency #521

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

Open
devmotion wants to merge 7 commits into master
base: master
Choose a base branch
Loading
from dw/testutils
Open

Make Test a weak dependency #521

devmotion wants to merge 7 commits into master from dw/testutils

Conversation

Copy link
Member

@devmotion devmotion commented Jun 26, 2023

I'm not sure if this is a good idea at all and if it works as intended - BUT I've encountered multiple repos in the last few months where people did not want to add dependencies on Test. So I was wondering if we could get away with making the test utilities an extension of the package and wanted to try it in KernelFunctions.

IMO, the main problems are that 1) the code is a bit more complex and contains a "workaround" (since only overloaded but not new functions in extensions are accessible for the user) and 2) the extension is always loaded when testing the PR in the REPL.

I assume the reason for 2) might be that the Test stdlib is included in the default system image (?). Or are stdlibs not supported as weakly dependencies (I guess @KristofferC should know this?)?

Copy link

codecov bot commented Jun 26, 2023
edited
Loading

Codecov Report

Attention: 74 lines in your changes are missing coverage. Please review.

Files Coverage Δ
src/KernelFunctions.jl 100.00% <ø> (ø)
src/TestUtils.jl 100.00% <100.00%> (+6.75%) ⬆️
ext/KernelFunctionsTestExt.jl 0.00% <0.00%> (ø)

... and 30 files with indirect coverage changes

📢 Thoughts on this report? Let us know!.

devmotion and others added 2 commits June 27, 2023 01:53
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Member Author

This approach was adopted by eg AbstractFFTs, TranscodingStreams, and (soon) Distributions. I think we should make Test a weak dependency to reduce the number of dependencies and to be nice to other packages that made Test a weak dependency.

devmotion and others added 2 commits October 20, 2023 10:40
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Copy link
Member

I agree that we should do this -- I'm contemplating doing it with some of my other repos as well anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /