Frequency settings are ignored for groups

[nicl]

cc @rtyley @akash1810

Issue raised on Scala Steward repo to get their thoughts too: scala-steward-org/scala-steward#3096.

The problem

We believe that our pullRequest.frequency settings are being ignored for grouped PRs, meaning AWS and non-AWS updates are occurring daily rather than weekly or monthly.

An attempted fix for this can be found here:

scala-steward-org/scala-steward#3087

It's unclear what the 'right' behaviour for Scala Steward is here and how grouping configuration should work together with dependency group overrides.

Groupings:

pullRequests.grouping = [
  { name = "aws", "title" = "AWS dependency updates", "filter" = [{"group" = "software.amazon.awssdk"}, {"group" = "com.amazonaws"}] },
  { name = "non_aws", "title" = "Non-AWS dependency updates", "filter" = [{"group" = "*"}] }
]

Dependency Maven 'groupId' frequency overrides:

dependencyOverrides = [
  {
    dependency = { groupId = "com.amazonaws" },
    pullRequests = { frequency = "7 days" }
  },
]

Relating dependency overrides with groupings is unclear because groupings support sophisticated filters, such as wildcards, whereas dependency overrides simply specific a groupId.

Really, it would be better if groups could contain their own frequency. E.g.

pullRequests.grouping = [
  { name = "aws", "title" = "AWS dependency updates", "filter" = [{"group" = "software.amazon.awssdk"}, {"group" = "com.amazonaws"}], frequency = "30 days" },
  { name = "non_aws", "title" = "Non-AWS dependency updates", "filter" = [{"group" = "*"}], frequency = "7 days" }
]

Solutions

At the moment, we see lots of PRs raised by Scala Steward:

• 2 * n * 7 per week (where 'n' is number of Scala repos)

Most of the PRs languish for a while before being closed or merged. At the time of writing we have:

• 189 open Scala Steward PRs on private repos
• 488 open Scala Steward PRs on public repos

(and 3368 open Dependabot ones, but that is another story)

Note: this data is out of date unfortunately as our Github Cloudquery job is failing, so let's fix that and update!

Let's be more realistic here and reduce the frequency of PRs to something more manageable. A suggested configuration is:

Option 1:

pullRequests.frequency = "7 days"

pullRequests.grouping = [
  { name = "all", "title" = "All dependency updates", "filter" = [{"group" = "*"}] },
]

I.e. just raise a single PR/week for each Scala repo. An order of magnitude reduction.

Option 2:

pullRequests.frequency = "7 days"

pullRequests.grouping = [
  { name = "patch", "title" = "Patch dependency updates", "filter" = [{"version" = "patch"}] },
  { name = "minor_major", "title" = "Major/minor dependency updates", "filter" = [{"version" = "minor"}, {"version" = "major"}] },
]

and settings patch to automerge somehow. Unfortunately semver isn't really a thing in Scala/Java land, hence we split by patch and other rather than major and other.

Suggestion is that we get some updated numbers on PRs raised, test out option 1 or 2 above, and then if happy roll out to our central config.

[rtyley]

It's unclear what the 'right' behaviour for Scala Steward is here and how grouping configuration should work together with dependency group overrides.

I disagree with that - I feel that what consistent behaviour would be is fairly clear (see scala-steward-org/scala-steward#3096 (comment) - it should just respect the specified dependency frequency) - so I'd be happy to fix that in Scala Steward!

How many Scala Steward PRs are open?

This is splitting hairs a bit, because scala-steward-org/scala-steward#3096 is a bug in Scala Steward that needs to get fixed, but I do think the numbers listed here are a bit misleading:

At the moment, we see lots of PRs raised by Scala Steward:

2 * n * 7 per week (where 'n' is number of Scala repos)
Most of the PRs languish for a while before being closed or merged. At the time of writing we have:

189 open Scala Steward PRs on private repos
488 open Scala Steward PRs on public repos

If we look at Scala Steward PRs on public repos, Scala Steward currently runs on 39 of our public repos. Additionally, given the grouping behaviour of Scala Steward adopted in #40, Scala Steward will only ever have 2 PRs open on any given repo (it doesn't, eg, have multiple open 'AWS dependency updates' PRs open at the same time for any given repo), so at most Scala Steward will only have 78 PRs (2*39) open at any given point in time. Of the ~500 open Scala Steward PRs on public repos, over 85% of those are legacy (ungrouped) PRs that just need to be closed.

The PR count doesn't go up by 2 * n * 7 per week, tho' potentially at least n*5 would get created per week if you chose to merge every AWS PR created by Scala Steward every workday.

What to do?

Of the solutions suggested above, Option 1 (changing from 2 groups to 1) potentially cuts the number of PRs by 50% - but given the nature of the bug underlying scala-steward-org/scala-steward#3096, I'm not certain that pullRequests.frequency = "7 days" will actually take effect? I also personally like having 2 groups, separating the noisy artifacts from the ones that have more intentional releases - for similar reasons I'm not that keen on Option 2. The PRs raised for noisy artifacts should have much lower frequencies, while promptness on artifacts with intentional releases is genuinely useful.

My preference is to get scala-steward-org/scala-steward#3096 fixed, can I take a crack at that before either Option 1 or 2 are applied to the global config?

[nicl]

@rtyley thanks! Two thoughts:

Option 1 should work from my reading of the Scala Steward code, though I need to check (see guardian/amigo#1205). As far as I can tell the bug only applies to frequencies set in dependencyOverrides but I could well have misunderstood this. I'm still not entirely convinced that combining frequency settings for specific group IDs (dependencyOverrides) and groupings is entirely sensible. I can imagine configurations which become very difficult to understand. But happy to accept that this is probably a 'bug' given the lack of docs against combining the two. Hopefully the maintainers will provide some clarity on the linked SS issue.

Re 2 * n * 7 it should perhaps be 'up to 2 * n * 5 PRs created a week. You are quite right that this only hits teams who merge PRs quickly - but that is precisely the behaviour we should be encouraging and not punishing! (And indeed, is what we aim to do in DevX, though not always successfully.) And 7 should become 5 - as you note, we only run Scala Steward on workdays. In any case, it is potentially a very high number.

[akash1810]

Summarising our IRL discussion on this:

Happy to try either option as they're both improvements on the current status quo.

We use a few tools to update dependencies; namely Scala Steward, and Dependabot. Each of them provide a vastly different DX.

Scala Steward raises PRs bumping groups of dependencies, whereas Dependabot raises a PR per dependency. As Scala Steward bumps in groups, it'll bump all dependencies, whereas we sometimes configure Dependabot to raise a maximum number of PRs at a time. Scala Steward (should) raise a PR once a week, whereas Dependabot is often configured to run once a month.

I wonder if a unified dependency updating story (and more automation) would increase the chance of PRs being reviewed and merged (we have a large number of open PRs)? For example, maybe https://github.com/marketplace/actions/combine-prs could be used for the batching functionality, and https://mergify.com/ for auto-merging?

[akash1810]

maybe https://github.com/marketplace/actions/combine-prs could be used for the batching functionality

As a trial, I've added this to guardian/master-to-main (see guardian/master-to-main#258 and guardian/master-to-main#262), with an output of guardian/master-to-main#261. It seems fairly straight forward, and worth rolling out.

[rtyley]

I think this specific issue (Frequency settings are ignored for groups) can now be closed, as happily Scala Steward v0.25.0 has been released, with the fix in PR scala-steward-org/scala-steward#3102 included - I've verified that this means dependencyOverrides-specific frequency settings are now respected for grouped-PRs - phew!

This means that these artifact groups will have reduced update frequency:

• com.amazonaws (7 days)
• software.amazon.awssdk & com.google.apis (30 days)

...and other artifacts can still trigger 'asap' updates (hopefully more consequential ones!)