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

Don't use UnionAlls in Varargs #658

Merged
merged 2 commits into from
Aug 9, 2022
Merged

Don't use UnionAlls in Varargs #658

merged 2 commits into from
Aug 9, 2022

Conversation

andreasnoack
Copy link
Member

This has been deprecated. I think it would be helpful to users to test with --depwarn=error but I'm not sure how to enable that when using julia-runtest. @DilumAluthge Do you know if that is possible?

@github-actions github-actions bot added the needs version bump Version needs to be incremented or set to -DEV in Project.toml label Aug 8, 2022
@devmotion
Copy link
Member

I think it would be helpful to users to test with --depwarn=error but I'm not sure how to enable that when using julia-runtest.

Apparently one can set depwarn to error: https://github.com/julia-actions/julia-runtest/blob/7ea3b3e0bd2f7de6d8104e238f69a715c988a5d1/action.yml#L16-L18 I.e., it seems one would have to use something like

uses: julia-actions/julia-runtest@v1
with:
  depwarn: error

@andreasnoack
Copy link
Member Author

Thanks. Let's try to see if it works.

@github-actions github-actions bot removed the needs version bump Version needs to be incremented or set to -DEV in Project.toml label Aug 8, 2022
@oxinabox
Copy link
Member

oxinabox commented Aug 8, 2022

I don't think we want depwarn=error.
It's deprecated.
It's not like it is going to be removed before Julia 2.0, right?

Further we do use some packages here, and those might deprecate things while we are still using them.
Sometimes using a deprecated thing is the best way to maximise compatibility.
Not in this case, but in general.
So I approve the core of this PR, but not the changes to our CI setup.

@andreasnoack
Copy link
Member Author

I've dropped the CI commit.

When deprecations were printed by default the overhead of taking the code path was often so high that it didn't matter that it technically wasn't breaking since the code ran too slowly to be useful. Apparently there is still a bit of overhead from taking these paths so it might be useful to detect them. We'll continue to do that downstream.

@andreasnoack andreasnoack merged commit 148fa88 into main Aug 9, 2022
@andreasnoack andreasnoack deleted the an/unionall branch August 9, 2022 08:45
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.

4 participants