Consider the extras block when checking for new versions
With the work in https://github.com/JuliaRegistries/CompatHelper.jl/pull/458, it seems like it's pretty simple to close #166 as also conjectured in that issue.
Should we make this toggleable? And should it be opt-in or opt-out?
Some thoughts:
- The main use case for
[extras]is test-only deps. Most packages in the ecosystem currently don't have[compat]entries for their test-only deps, so this would lead to a surge of new PRs across the ecosystem. - AutoMerge requires packages to have
[compat]entries for all regular deps. However, AutoMerge, doesn't currently require packages to have[compat]entries for test-only deps[^1]. CompatHelper was originally invented for the purpose of helping users meet the AutoMerge requirements. In some sense, that continues to be a core purpose of CompatHelper, so perhaps it makes sense that for the default behavior to be congruent with what AutoMerge requires (with of course the ability for users to opt-in to additional functionalities).
[^1]: Maybe we should add that requirement, but that's likely a discussion for a different day.
There is indeed a significant difference between direct package dependencies and test dependencies in that the latter can't break the environment alone, so having wrong compat (e.g. no) bounds on test dependencies could be considered a private matter. However, I don't really see why it was ever a good idea not to have compat bounds on test dependencies. It's not at all obvious that new versions of test dependencies wouldn't break your tests. However, bottomline here is probably that it should be opt-in. This will require a little more work on the PR, though.
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 100.00%. Comparing base (7ea4330) to head (1aea258).
:warning: Report is 5 commits behind head on master.
Additional details and impacted files
@@ Coverage Diff @@
## master #502 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 12 12
Lines 412 410 -2
=========================================
- Hits 412 410 -2
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
@DilumAluthge has anything changed with respect to
AutoMerge, doesn't currently require packages to have [compat] entries for test-only deps
or is it status quo?
Nothing has changed, but we might consider making a breaking release of CompatHelper in which we start enforcing compat bounds for test deps (as the default behavior; we could add a kwarg that allows users to opt-out)?
IMO it's important to file PRs to adjust bounds for extra deps by default. I just bumped into this case in StatsModels (https://github.com/JuliaStats/StatsModels.jl/pull/323) with CategoricalArrays 1.0. If CompatHelper doesn't notice developers that a new version of the dependency has been released, the package CI will most likely continue testing an old version forever, while most users will use the new one.
To minimize the number of new PRs, would it be possible to only track new versions when version bounds are specified for a given package?
I agree, I also just ran into this with CategoricalArrays 1.0 - tests were still running on an older version and CompatHelper didn't notify me about the release.
@DilumAluthge what's the blocker for the PR? That it would be a breaking change? Maybe it could be softened by a) only opening CompatHelper PRs for dependencies for which already a version bound exists (as suggested in the last comment) and/or b) making it an opt-in feature initially.
Maybe it could be softened by a) only opening CompatHelper PRs for dependencies for which already a version bound exists (as suggested in the last comment) and/or b) making it an opt-in feature initially.
I think either of those would be a good idea.
How about an optional kwarg named open_prs_for_extras::Symbol, with the following allowed values:
:all- will open PRs for all[extras]:none- won't open any PRs for[extras]:if_existing_compat- if there's an existing compat entry, we'll open PRs, but if there's no existing compat entry, then we won't open a PR.
The kwarg would be optional, and we'd default to :if_existing_compat.
Also, we'd throw an error if any other value (other than the above three values) was passed as the value of open_prs_for_extras.
I extended the PR in https://github.com/JuliaRegistries/CompatHelper.jl/pull/528 along these lines.
Closing in favor of #528.