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

Adapt to Julia 1.11 #50

Merged
merged 2 commits into from
Nov 23, 2024
Merged

Adapt to Julia 1.11 #50

merged 2 commits into from
Nov 23, 2024

Conversation

gdalle
Copy link
Contributor

@gdalle gdalle commented Nov 23, 2024

Hi there, I was looking at ways to tackle #49 and I thought that if you want to use DI, we might as well upgrate to Julia 1.10 because I dropped 1.6 when the new LTS was announced.
Here are a few changes, feel free to take them or leave them:

  • Bump Julia compat to 1.10
  • Update CI to run on 3 automatically updated versions: lts, 1 (stable release) and pre (upcoming pre-release)
  • Remove Requires-related extension boilerplate (but leave default backend choice)

Copy link

codecov bot commented Nov 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.02%. Comparing base (b8c3464) to head (5a6197a).
Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #50      +/-   ##
==========================================
+ Coverage   95.46%   96.02%   +0.56%     
==========================================
  Files          21       22       +1     
  Lines         375      428      +53     
==========================================
+ Hits          358      411      +53     
  Misses         17       17              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@kellertuer
Copy link
Member

kellertuer commented Nov 23, 2024

Thanks for the PR!

I think my only hesitation would be, that this would be the very first package in our ecosystem to drop 1.6-1.9 support.
On the other hand, sure, the extensions code gets much nicer when dropping 1.8 support.

I like the idea with its, 1, and pre, nice and generic.

@gdalle
Copy link
Contributor Author

gdalle commented Nov 23, 2024

If that version of the language itself is not maintained, should package developers really be expected to keep it alive?
Note that this change wouldn't prevent users from installing ManifoldDiff on Julia 1.6, they would just get the last compatible version, in this case 0.3.13.
In any case, I no longer support Julia 1.6 on DI, hence this PR to test the waters before #49. But there's no rush.

@kellertuer
Copy link
Member

That sounds like a fair argument, sure.
And yeah, maybe a main reason we never went to 1.10 as lower bound was really only, that there was no good reason to.
And sure, then this plan here is.

Copy link
Member

@mateuszbaran mateuszbaran left a comment

Choose a reason for hiding this comment

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

I'm also fine with bumping Julia requirement 1.10

@kellertuer
Copy link
Member

Nice. @gdalle is it ok if we merge this on master but only register it together with the addition of DI?

@gdalle
Copy link
Contributor Author

gdalle commented Nov 23, 2024

Sure, but note that I bumped the version to v0.3.14 because dropping an old Julia version is not breaking. On the other hand, DI integration will probably be breaking and require a bump to v0.4.0

@kellertuer
Copy link
Member

Then 0.3.14 can still chill unregistered on main - and if you have a breaking PR for DI, then it chilled but never got registered. That would be fine with me.

@gdalle
Copy link
Contributor Author

gdalle commented Nov 23, 2024

Sounds good. Plus it allows you to make other changes and register them before the DI PR comes along

@kellertuer
Copy link
Member

Yes – great.

@kellertuer kellertuer merged commit cbee92a into JuliaManifolds:main Nov 23, 2024
15 checks passed
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