Conversation
09f613e to
0489081
Compare
83d4acb to
8a8d2fe
Compare
147aee0 to
bd00bbb
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3787 +/- ##
==========================================
+ Coverage 89.83% 89.88% +0.05%
==========================================
Files 174 174
Lines 5036 5063 +27
Branches 458 447 -11
==========================================
+ Hits 4524 4551 +27
Misses 512 512 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
f2bcc87 to
995c9b1
Compare
Update.Single types to just nextVersion, not newerVersions - and avoid use of Update.ForArtifactId when evaluating candidate versions
45ca0c0 to
972777f
Compare
rtyley
added a commit
that referenced
this pull request
Jan 16, 2026
These are some small tweaks I've extracted from this larger PR, to take the size down a bit: #3787
1e2253b to
943b62b
Compare
Update.Single types to just nextVersion, not newerVersions - and avoid use of Update.ForArtifactId when evaluating candidate versionsUpdates (where the single nextVersion has been determined)
Updates (where the single nextVersion has been determined)newerVersions from Updates (where the single nextVersion has been determined)
newerVersions from Updates (where the single nextVersion has been determined)newerVersions from Updates
9f21f95 to
28c68e6
Compare
mzuehlke
approved these changes
Jan 18, 2026
Member
mzuehlke
left a comment
There was a problem hiding this comment.
I like these more specific classes.
Everyone will benefit from easier code understanding-
_Addresses https://github.com/scala-steward-org/scala-steward/issues/3781_ The Scala Steward codebase deals with the concept of 'updates' in two areas: * **Multiple `newerVersions`**: Initially, evaluating _candidate_ updates (in `UpdateAlg` & `FilterAlg`), where for any given artifact, there can be many possible versions we might want to upgrade to. The list of versions is reduced down with filtering (based on config & other rules), and eventually a single next version is selected. * **Single `nextVersion`**: Subsequently, retrieving information on the updates represented by the pull requests Scala Steward created - each artifact being updated to a specific _single_ new version. ...Scala Steward used the `Update` type for _both_ areas (specifically, `Update.ForArtifactId` for the first phase, _requiring_ it to support multiple `newerVersions`). Looking at a single `Update.ForArtifactId` in code, it's not always clear to the newcomer if the update contains many newer versions or not - ie where we are in the pipeline. ## Changes * `Update.ForArtifactId` → `ArtifactUpdateCandidates`🆕 within `UpdateAlg` & `FilterAlg` where candidate versions are being evaluated * `Update.ForArtifactId` & `ArtifactUpdateCandidates` both extend `ArtifactUpdateVersions`🆕 to allow `UpdatePattern.findMatch()` to filter lists of either of them: * Filtering the candidate versions of `ArtifactUpdateCandidates` in `UpdatesConfig.isAllowed`, etc * Filtering the `Update`s of existing PRs in `PruningAlg.newPullRequestsAllowed()` & `RetractedArtifact.isRetracted()` * The fields from `Update.ForArtifactId` also required by `ArtifactUpdateCandidates` have been placed in the `ArtifactForUpdate`🆕 case class * Restrict `Update.Single` types to just `nextVersion`, not `newerVersions` Remaining unchanged: * `PullRequestRepository` still persists instances of `Update` * The JSON serialisation format of `Update` remains unchanged for now (the encoder & decoders have been adapted so that they still output the same format, despite the case classes having changed), so there's currently no need to introduce a new fallback 'legacy' decoder (several of which were recently deleted with #3785). This also means the JSON metadata added to PR descriptions (originally introduced in #3466) also remains the same for now, though I think it will be reasonable to adjust the format for `Update.ForArtifactId` to something more logical in future.
28c68e6 to
df28ac4
Compare
rtyley
added a commit
that referenced
this pull request
Jan 18, 2026
Add deserialisation test for VersionsCache Value Add json fixture for new Value type Add VersionWithFirstSeen Add merging of new and old first-seen version data Add deserialisation test for new format It looks like we probably want to update UpdatesConfig.scala to add a new method `isTooNew: Update.ForArtifactId => FilterResult` which rules out proposed updates that use too new a version (according to the user’s config). To do that, we reckon we’ll need Update.ForArtifactId to list versions with their firstSeen information, i.e. update the contained type from Version to VersionWithFirstSeen. This commit begins making that change (and so will not compile). Replace withNewerVersions with supersedes This commit removes withNewerVersions and replaces the one usage of it with a new function supersedes, which checks whether an update matches another while having a newer nextVersion. withNewerVersions was a little awkward to update to work with us having changed some Versions to VersionWithFirstSeens, because it accepts a list of Versions (rather than just producing one). It was originally added in response to [this PR comment](#1667 (comment)), but we reckon replacing it with a slightly different check of the group and artifact IDs makes for clearer, more explicit code. We think (though it’s somewhat hard to tell) that this doesn’t remove any existing functionality: part of the argument for adding withNewerVersions in the first place was to support Update.Group, but it appears to only be called for Update.Singles. Add CooldownConfig To facilitate using dependency cooldowns, this commit adds a class CooldownConfig to represent a user’s desired cooldown settings: a list of ages (specified as finite durations, using the same string syntax as supported for pullRequests.frequency), each with a list of UpdatePatterns for the age to apply to. Co-authored-by: Roberto Tyley <roberto.tyley@guardian.co.uk>
rtyley
added a commit
that referenced
this pull request
Jan 18, 2026
Add deserialisation test for VersionsCache Value Add json fixture for new Value type Add VersionWithFirstSeen Add merging of new and old first-seen version data Add deserialisation test for new format It looks like we probably want to update UpdatesConfig.scala to add a new method `isTooNew: Update.ForArtifactId => FilterResult` which rules out proposed updates that use too new a version (according to the user’s config). To do that, we reckon we’ll need Update.ForArtifactId to list versions with their firstSeen information, i.e. update the contained type from Version to VersionWithFirstSeen. This commit begins making that change (and so will not compile). Replace withNewerVersions with supersedes This commit removes withNewerVersions and replaces the one usage of it with a new function supersedes, which checks whether an update matches another while having a newer nextVersion. withNewerVersions was a little awkward to update to work with us having changed some Versions to VersionWithFirstSeens, because it accepts a list of Versions (rather than just producing one). It was originally added in response to [this PR comment](#1667 (comment)), but we reckon replacing it with a slightly different check of the group and artifact IDs makes for clearer, more explicit code. We think (though it’s somewhat hard to tell) that this doesn’t remove any existing functionality: part of the argument for adding withNewerVersions in the first place was to support Update.Group, but it appears to only be called for Update.Singles. Add CooldownConfig To facilitate using dependency cooldowns, this commit adds a class CooldownConfig to represent a user’s desired cooldown settings: a list of ages (specified as finite durations, using the same string syntax as supported for pullRequests.frequency), each with a list of UpdatePatterns for the age to apply to. Co-authored-by: Roberto Tyley <roberto.tyley@guardian.co.uk>
rtyley
added a commit
that referenced
this pull request
Jan 18, 2026
Add deserialisation test for VersionsCache Value Add json fixture for new Value type Add VersionWithFirstSeen Add merging of new and old first-seen version data Add deserialisation test for new format It looks like we probably want to update UpdatesConfig.scala to add a new method `isTooNew: Update.ForArtifactId => FilterResult` which rules out proposed updates that use too new a version (according to the user’s config). To do that, we reckon we’ll need Update.ForArtifactId to list versions with their firstSeen information, i.e. update the contained type from Version to VersionWithFirstSeen. This commit begins making that change (and so will not compile). Replace withNewerVersions with supersedes This commit removes withNewerVersions and replaces the one usage of it with a new function supersedes, which checks whether an update matches another while having a newer nextVersion. withNewerVersions was a little awkward to update to work with us having changed some Versions to VersionWithFirstSeens, because it accepts a list of Versions (rather than just producing one). It was originally added in response to [this PR comment](#1667 (comment)), but we reckon replacing it with a slightly different check of the group and artifact IDs makes for clearer, more explicit code. We think (though it’s somewhat hard to tell) that this doesn’t remove any existing functionality: part of the argument for adding withNewerVersions in the first place was to support Update.Group, but it appears to only be called for Update.Singles. Add CooldownConfig To facilitate using dependency cooldowns, this commit adds a class CooldownConfig to represent a user’s desired cooldown settings: a list of ages (specified as finite durations, using the same string syntax as supported for pullRequests.frequency), each with a list of UpdatePatterns for the age to apply to. Co-authored-by: Roberto Tyley <roberto.tyley@guardian.co.uk>
Contributor
Author
Many thanks @mzuehlke ! |
rtyley
pushed a commit
that referenced
this pull request
Jan 20, 2026
Add deserialisation test for VersionsCache Value Add json fixture for new Value type Add VersionWithFirstSeen Add merging of new and old first-seen version data Add deserialisation test for new format It looks like we probably want to update UpdatesConfig.scala to add a new method `isTooNew: Update.ForArtifactId => FilterResult` which rules out proposed updates that use too new a version (according to the user’s config). To do that, we reckon we’ll need Update.ForArtifactId to list versions with their firstSeen information, i.e. update the contained type from Version to VersionWithFirstSeen. This commit begins making that change (and so will not compile). Replace withNewerVersions with supersedes This commit removes withNewerVersions and replaces the one usage of it with a new function supersedes, which checks whether an update matches another while having a newer nextVersion. withNewerVersions was a little awkward to update to work with us having changed some Versions to VersionWithFirstSeens, because it accepts a list of Versions (rather than just producing one). It was originally added in response to [this PR comment](#1667 (comment)), but we reckon replacing it with a slightly different check of the group and artifact IDs makes for clearer, more explicit code. We think (though it’s somewhat hard to tell) that this doesn’t remove any existing functionality: part of the argument for adding withNewerVersions in the first place was to support Update.Group, but it appears to only be called for Update.Singles. Add CooldownConfig To facilitate using dependency cooldowns, this commit adds a class CooldownConfig to represent a user’s desired cooldown settings: a list of ages (specified as finite durations, using the same string syntax as supported for pullRequests.frequency), each with a list of UpdatePatterns for the age to apply to. Co-authored-by: Roberto Tyley <roberto.tyley@guardian.co.uk> # Conflicts: # modules/core/src/main/scala/org/scalasteward/core/data/Update.scala
rtyley
pushed a commit
that referenced
this pull request
Jan 20, 2026
Add deserialisation test for VersionsCache Value Add json fixture for new Value type Add VersionWithFirstSeen Add merging of new and old first-seen version data Add deserialisation test for new format It looks like we probably want to update UpdatesConfig.scala to add a new method `isTooNew: Update.ForArtifactId => FilterResult` which rules out proposed updates that use too new a version (according to the user’s config). To do that, we reckon we’ll need Update.ForArtifactId to list versions with their firstSeen information, i.e. update the contained type from Version to VersionWithFirstSeen. This commit begins making that change (and so will not compile). Replace withNewerVersions with supersedes This commit removes withNewerVersions and replaces the one usage of it with a new function supersedes, which checks whether an update matches another while having a newer nextVersion. withNewerVersions was a little awkward to update to work with us having changed some Versions to VersionWithFirstSeens, because it accepts a list of Versions (rather than just producing one). It was originally added in response to [this PR comment](#1667 (comment)), but we reckon replacing it with a slightly different check of the group and artifact IDs makes for clearer, more explicit code. We think (though it’s somewhat hard to tell) that this doesn’t remove any existing functionality: part of the argument for adding withNewerVersions in the first place was to support Update.Group, but it appears to only be called for Update.Singles. Add CooldownConfig To facilitate using dependency cooldowns, this commit adds a class CooldownConfig to represent a user’s desired cooldown settings: a list of ages (specified as finite durations, using the same string syntax as supported for pullRequests.frequency), each with a list of UpdatePatterns for the age to apply to. Co-authored-by: Roberto Tyley <roberto.tyley@guardian.co.uk> # Conflicts: # modules/core/src/main/scala/org/scalasteward/core/data/Update.scala
rtyley
pushed a commit
to guardian/scala-steward
that referenced
this pull request
Jan 23, 2026
Add deserialisation test for VersionsCache Value Add json fixture for new Value type Add VersionWithFirstSeen Add merging of new and old first-seen version data Add deserialisation test for new format It looks like we probably want to update UpdatesConfig.scala to add a new method `isTooNew: Update.ForArtifactId => FilterResult` which rules out proposed updates that use too new a version (according to the user’s config). To do that, we reckon we’ll need Update.ForArtifactId to list versions with their firstSeen information, i.e. update the contained type from Version to VersionWithFirstSeen. This commit begins making that change (and so will not compile). Replace withNewerVersions with supersedes This commit removes withNewerVersions and replaces the one usage of it with a new function supersedes, which checks whether an update matches another while having a newer nextVersion. withNewerVersions was a little awkward to update to work with us having changed some Versions to VersionWithFirstSeens, because it accepts a list of Versions (rather than just producing one). It was originally added in response to [this PR comment](scala-steward-org#1667 (comment)), but we reckon replacing it with a slightly different check of the group and artifact IDs makes for clearer, more explicit code. We think (though it’s somewhat hard to tell) that this doesn’t remove any existing functionality: part of the argument for adding withNewerVersions in the first place was to support Update.Group, but it appears to only be called for Update.Singles. Add CooldownConfig To facilitate using dependency cooldowns, this commit adds a class CooldownConfig to represent a user’s desired cooldown settings: a list of ages (specified as finite durations, using the same string syntax as supported for pullRequests.frequency), each with a list of UpdatePatterns for the age to apply to. Co-authored-by: Roberto Tyley <roberto.tyley@guardian.co.uk> # Conflicts: # modules/core/src/main/scala/org/scalasteward/core/data/Update.scala # Conflicts: # modules/core/src/main/scala/org/scalasteward/core/update/FilterAlg.scala
rtyley
pushed a commit
to guardian/scala-steward
that referenced
this pull request
Jan 29, 2026
Add deserialisation test for VersionsCache Value Add json fixture for new Value type Add VersionWithFirstSeen Add merging of new and old first-seen version data Add deserialisation test for new format It looks like we probably want to update UpdatesConfig.scala to add a new method `isTooNew: Update.ForArtifactId => FilterResult` which rules out proposed updates that use too new a version (according to the user’s config). To do that, we reckon we’ll need Update.ForArtifactId to list versions with their firstSeen information, i.e. update the contained type from Version to VersionWithFirstSeen. This commit begins making that change (and so will not compile). Replace withNewerVersions with supersedes This commit removes withNewerVersions and replaces the one usage of it with a new function supersedes, which checks whether an update matches another while having a newer nextVersion. withNewerVersions was a little awkward to update to work with us having changed some Versions to VersionWithFirstSeens, because it accepts a list of Versions (rather than just producing one). It was originally added in response to [this PR comment](scala-steward-org#1667 (comment)), but we reckon replacing it with a slightly different check of the group and artifact IDs makes for clearer, more explicit code. We think (though it’s somewhat hard to tell) that this doesn’t remove any existing functionality: part of the argument for adding withNewerVersions in the first place was to support Update.Group, but it appears to only be called for Update.Singles. Add CooldownConfig To facilitate using dependency cooldowns, this commit adds a class CooldownConfig to represent a user’s desired cooldown settings: a list of ages (specified as finite durations, using the same string syntax as supported for pullRequests.frequency), each with a list of UpdatePatterns for the age to apply to. Co-authored-by: Roberto Tyley <roberto.tyley@guardian.co.uk> # Conflicts: # modules/core/src/main/scala/org/scalasteward/core/data/Update.scala # Conflicts: # modules/core/src/main/scala/org/scalasteward/core/update/FilterAlg.scala
rtyley
pushed a commit
to guardian/scala-steward
that referenced
this pull request
Jan 29, 2026
Add deserialisation test for VersionsCache Value Add json fixture for new Value type Add VersionWithFirstSeen Add merging of new and old first-seen version data Add deserialisation test for new format It looks like we probably want to update UpdatesConfig.scala to add a new method `isTooNew: Update.ForArtifactId => FilterResult` which rules out proposed updates that use too new a version (according to the user’s config). To do that, we reckon we’ll need Update.ForArtifactId to list versions with their firstSeen information, i.e. update the contained type from Version to VersionWithFirstSeen. This commit begins making that change (and so will not compile). Replace withNewerVersions with supersedes This commit removes withNewerVersions and replaces the one usage of it with a new function supersedes, which checks whether an update matches another while having a newer nextVersion. withNewerVersions was a little awkward to update to work with us having changed some Versions to VersionWithFirstSeens, because it accepts a list of Versions (rather than just producing one). It was originally added in response to [this PR comment](scala-steward-org#1667 (comment)), but we reckon replacing it with a slightly different check of the group and artifact IDs makes for clearer, more explicit code. We think (though it’s somewhat hard to tell) that this doesn’t remove any existing functionality: part of the argument for adding withNewerVersions in the first place was to support Update.Group, but it appears to only be called for Update.Singles. Add CooldownConfig To facilitate using dependency cooldowns, this commit adds a class CooldownConfig to represent a user’s desired cooldown settings: a list of ages (specified as finite durations, using the same string syntax as supported for pullRequests.frequency), each with a list of UpdatePatterns for the age to apply to. Co-authored-by: Roberto Tyley <roberto.tyley@guardian.co.uk> # Conflicts: # modules/core/src/main/scala/org/scalasteward/core/data/Update.scala # Conflicts: # modules/core/src/main/scala/org/scalasteward/core/update/FilterAlg.scala
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses #3781 -
Update.Singledoesn't neednewerVersions, justnextVersionThe Scala Steward codebase deals with the concept of 'updates' in two areas:
newerVersions: Initially, evaluating candidate updates (inUpdateAlg&FilterAlg), where for any given artifact, there can be many possible versions we might want to upgrade to. The list of versions is reduced down with filtering (based on config & other rules), and eventually a single next version is selected.nextVersion: Subsequently, retrieving information on the updates represented by the pull requests Scala Steward created - each artifact being updated to a specific single new version....Scala Steward used the
Updatetype for both areas (particularly,Update.ForArtifactIdfor the first phase, requiring it to support multiplenewerVersions).Looking at a single
Update.ForArtifactIdin code, it's not always clear to the newcomer if the update contains many newer versions or not - ie where we are in the pipeline.Changes
Update.Singletypes (egUpdate.ForArtifactId) now just have anextVersionfield, notnewerVersions, which better models what they actually signify: the single specific update selected for a particular PRUpdateAlg&FilterAlgwe changeUpdate.ForArtifactId→ArtifactUpdateCandidates🆕 (which does have anewerVersionsfield) for the parts where multiple candidate versions are being evaluatedUpdate.ForArtifactId&ArtifactUpdateCandidatesboth extendArtifactUpdateVersions🆕 to allowUpdatePattern.findMatch()to filter lists of either of them:ArtifactUpdateCandidatesinUpdatesConfig.isAllowed, etcUpdates of existing PRs inPruningAlg.newPullRequestsAllowed()&RetractedArtifact.isRetracted()Update.ForArtifactIdalso required byArtifactUpdateCandidateshave been placed in theArtifactForUpdate🆕 case class, avoiding possible duplicationRemaining unchanged:
PullRequestRepositorystill persists instances ofUpdateUpdateremains unchanged for now (the encoder & decoders have been adapted so that they still output the same format, despite the case classes having changed, just with a single 'newerVersion'), so there's currently no need to introduce a new fallback 'legacy' decoder (several of which were recently deleted with Remove legacyUpdatedecoders (drop support for old PR caches) #3785). This also means the JSON metadata added to PR descriptions (originally introduced in Add JSON metadata in PRs #3466) also remains the same for now, though I think it will be reasonable to adjust the format forUpdate.ForArtifactIdto something more logical in future.With this PR, the changes from #3762 will no longer need to change the data stored in
PullRequestRepository.