-
Notifications
You must be signed in to change notification settings - Fork 32
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
Fix Aqua issue #312
Fix Aqua issue #312
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #312 +/- ##
=======================================
Coverage 96.58% 96.58%
=======================================
Files 15 15
Lines 1055 1055
=======================================
Hits 1019 1019
Misses 36 36 ☔ View full report in Codecov by Sentry. |
(LinearOperators) pkg> st
ERROR: Compat `ChainRulesCore` not listed in `deps` or `extras` section. Should I move it to extras? |
@geoffroyleconte I wasn't sure, so I tried something. The hack to keep maintaining Julia 1.6 is to keep the extras section for weak deps. |
That's a bit weird to have a separate .toml in the tests no? We could keep my initial commit, I don't think there is an effect on performance? |
I know Julia have pushing for this for a while, see https://pkgdocs.julialang.org/v1/creating-packages/#Adding-tests-to-the-package for instance even though both approaches are still supported. I think it helps maintability to keep compatibility of versions used (even though it means we have to update these compatibility constraints sometimes). No clue for the documentation, it seems everytime a PR has more than one commit it does this... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. Just to make sure you have seen this line in the link you provided:
"
The exact interaction between Project.toml, test/Project.toml and their corresponding Manifest.tomls are not fully worked out and may be subject to change in future versions. The old method of adding test-specific dependencies, described in the next section, will therefore be supported throughout all Julia 1.X releases.
"
If that is not an issue you can merge (I cannot approve my own PR).
Yes, it's true, and it seems this comment is here since version 1.2... but I still think it makes sense to have compat for tests. Even more now, that base packages have versions. |
There are issues with Aqua because there are no deps declared for ChainRulesCore:
@tmigot