Skip to content

Conversation

@odow
Copy link
Member

@odow odow commented Nov 13, 2023

This is a WIP PR to flesh out what a refactored SCS.jl would look like if we used package extensions.

CI will be broken until at least

@kalmarek
Copy link
Collaborator

@odow looks rather good, no more requires, init etc, much cleaner;

I'd suggest however to wait with this until the a new LTS is released which does support extensions. In some sense the current situation, while not optimal just works, and I don't feel like putting more burden of maintaining two incompatible versions of SCS.jl

@codecov
Copy link

codecov bot commented Nov 14, 2023

Codecov Report

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

Comparison is base (79917cc) 90.35% compared to head (24824ad) 87.87%.

Files Patch % Lines
ext/SCSSCS_GPU_jllExt.jl 0.00% 14 Missing ⚠️
ext/SCSSCS_MKL_jllExt.jl 85.71% 2 Missing ⚠️
src/SCS.jl 83.33% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #286      +/-   ##
==========================================
- Coverage   90.35%   87.87%   -2.48%     
==========================================
  Files           8        8              
  Lines         425      429       +4     
==========================================
- Hits          384      377       -7     
- Misses         41       52      +11     

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

@odow odow changed the title WIP: [breaking] refactor linear solver loading to use Pkg extensions [breaking] refactor linear solver loading to use Pkg extensions Nov 14, 2023
@odow
Copy link
Member Author

odow commented Nov 14, 2023

@kalmarek I'm trying to sort it so that we support @requires on 1.6 and the package extensions on 1.9.

We'll still have to do a breaking change to the use package loading, but that's okay. When 1.9 becomes the LTS, we can drop the requires part.

Copy link
Collaborator

@kalmarek kalmarek left a comment

Choose a reason for hiding this comment

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

Looks great!

@odow
Copy link
Member Author

odow commented Nov 14, 2023

cc @blegat you should take a look at this. Nominally it's a breaking change, but it affects only the lazy loading of the MKL and GPU solvers.

@blegat
Copy link
Member

blegat commented Nov 20, 2023

Great work, it's much cleaner now

@odow odow merged commit 062ac53 into master Nov 20, 2023
@odow odow deleted the od/ext branch November 20, 2023 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants