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

Revert "Drop support for julia 1.0-1.3 (#221)" #224

Merged
merged 5 commits into from
Nov 5, 2023
Merged

Conversation

lgoettgens
Copy link
Collaborator

No description provided.

@codecov
Copy link

codecov bot commented Oct 27, 2023

Codecov Report

Merging #224 (ddf0fda) into master (de25cfa) will decrease coverage by 12.17%.
Report is 1 commits behind head on master.
The diff coverage is 51.73%.

@@             Coverage Diff             @@
##           master     #224       +/-   ##
===========================================
- Coverage   88.33%   76.17%   -12.17%     
===========================================
  Files          10       11        +1     
  Lines         506      768      +262     
===========================================
+ Hits          447      585      +138     
- Misses         59      183      +124     
Flag Coverage Δ
unittests 76.17% <51.73%> (-12.17%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
src/Aqua.jl 73.07% <ø> (ø)
src/persistent_tasks.jl 87.71% <100.00%> (+0.44%) ⬆️
src/pkg/Versions.jl 51.36% <51.36%> (ø)

... and 3 files with indirect coverage changes

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@lgoettgens
Copy link
Collaborator Author

As already expected, `Test = "1.0" fails on julia 1.0-1.3 (log)

@lgoettgens
Copy link
Collaborator Author

Test = "<0.0.1, 1" indeed seems to work. Thanks @DilumAluthge!

@lgoettgens lgoettgens requested a review from fingolfin October 27, 2023 08:52
@lgoettgens lgoettgens marked this pull request as ready for review October 27, 2023 08:54
@lgoettgens
Copy link
Collaborator Author

@fingolfin What do you think? Should we enable compatibility with julia 1.0 again (by merging this PR)? If yes, I should look through the changes in detail, because there were a few conflicts when reverting stuff.

If you approve this, could you change the required CI jobs back to needing 1.0-1.3 for ubuntu, 1.0 for windows and macOS, and instead remove 1.4 for windows and macOS?

@hyrodium
Copy link
Contributor

I believe it is important that the code written for Julia 1.0 works with the latest versions of Julia and some packages. On the other hand, I think it is rare for users who do not upgrade their Julia version to only upgrade the version of Aqua.jl, so I feel there is no need to actively support Julia versions prior to 1.6 (LTS).

Sorry for repeating my previous comment (#216 (comment)), but I feel it would be nice to reduce the costs to maintain this package.

Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

Honestly I don't think support for the older Julia versions is worth the hassle.

However, if you are willing to take care of the burden, I certainly won't object! In the worst case, we could still re-remove it later.

As such, I think you should make the final decision, Lars. I won't complain either way :-)

@lgoettgens lgoettgens changed the title Experiment with Test compat for julia 1.0-1.3 Revert "Drop support for julia 1.0-1.3 (#221)" Nov 3, 2023
@lgoettgens
Copy link
Collaborator Author

I would like to continue with this PR. As said, we can drop support again at any point in time, if maintaining becomes too much of a hassle.

If you approve this, could you change the required CI jobs back to needing 1.0-1.3 for ubuntu, 1.0 for windows and macOS, and instead remove 1.4 for windows and macOS?

@fingolfin could you please adapt the "required" CI jobs as listed here? And feel free to merge once that is done.

src/persistent_tasks.jl Outdated Show resolved Hide resolved
@fingolfin fingolfin merged commit 3ea2897 into master Nov 5, 2023
21 of 24 checks passed
@fingolfin fingolfin deleted the lg/Test-compat branch November 5, 2023 00:04
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.

3 participants