Consider the extras block when checking for new versions

[andreasnoack]

With the work in #458, it seems like it's pretty simple to close #166 as also conjectured in that issue.

[DilumAluthge]

Should we make this toggleable? And should it be opt-in or opt-out?

[DilumAluthge]

Some thoughts:

1. 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.
2. 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¹. 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).

Footnotes

1. Maybe we should add that requirement, but that's likely a discussion for a different day. ↩

[andreasnoack]

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]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (7ea4330) to head (1aea258).

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     

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

[andreasnoack]

@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?

[DilumAluthge]

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)?

[nalimilan]

IMO it's important to file PRs to adjust bounds for extra deps by default. I just bumped into this case in StatsModels (JuliaStats/StatsModels.jl#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?

[devmotion]

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.

[DilumAluthge]

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.

[DilumAluthge]

How about an optional kwarg named open_prs_for_extras::Symbol, with the following allowed values:

1. :all - will open PRs for all [extras]
2. :none - won't open any PRs for [extras]
3. :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.

[devmotion]

I extended the PR in #528 along these lines.