Skip to content

[Updater] Extract RefreshVersionPullRequest as an Operation class#6939

Merged
brrygrdn merged 8 commits intomainfrom
brrygrdn/extract-refresh-version-update-operation
Mar 31, 2023
Merged

[Updater] Extract RefreshVersionPullRequest as an Operation class#6939
brrygrdn merged 8 commits intomainfrom
brrygrdn/extract-refresh-version-update-operation

Conversation

@brrygrdn
Copy link
Copy Markdown
Contributor

@brrygrdn brrygrdn commented Mar 29, 2023

Follows on from #6866, #6884

This branch peels out the Updater code required to accept a job which is "refreshing" an existing Dependabot PR for a version update using the same approach as before:

  • Extract the entire code path required from the Updater
  • Remove any code related to non-concerns for this operation type, in this case security-updates primarily
  • Wire it into the Updater to be used for a specific job type, verifying current testing passes

Why Refresh?

We use different terminology for this operation in some places depending on the trigger, but it isn't correct to think of it as a "rebase" despite commands like @dependabot rebase existing.

The actual behaviour is one of three outcomes:

  • If the project still needs the same dependency at the same version, then the PR is updates on top of the current head of the target branch; this is effectively a rebase
  • If the project still needs the same dependency updated, but there is a newer version, then a new PR is created to supersede the existing one
  • If a number of other circumstances have changed, such as the Dependency now being removed, ignored, updated by another commit, etc, then the PR is closed.

The term "update" is fairly saturated in Dependabot for obvious reasons, so I went with "refresh" as a good-enough catch all term.

Notes

As with #6866, this code is intentionally not DRY'd out as much as possible. We've de-risked this approach and this branch completes the rule of three, so the next step will be push the shared code into an interface mixin that is common to all Operation classes before starting to peel out the two Security Operations.

This PR is also branched off #6934 which should ship first as an enabling change.

@brrygrdn brrygrdn requested a review from a team as a March 29, 2023 20:54
@brrygrdn brrygrdn force-pushed the brrygrdn/update-dependency-list-early branch from e1642cc to 1ee7a5c Compare March 30, 2023 12:27
@brrygrdn brrygrdn force-pushed the brrygrdn/extract-refresh-version-update-operation branch from 491aad5 to 2096c14 Compare March 30, 2023 12:30
Base automatically changed from brrygrdn/update-dependency-list-early to main March 30, 2023 12:36
@brrygrdn brrygrdn force-pushed the brrygrdn/extract-refresh-version-update-operation branch 3 times, most recently from 933bd79 to 99adc98 Compare March 31, 2023 10:52
@brrygrdn
Copy link
Copy Markdown
Contributor Author

brrygrdn commented Mar 31, 2023

@landongrindheim I've added a last commit removing some dead code from the def dependencies method for the new operation - this operation cannot be invoked if job.dependencies is empty, so we will always early return the outcome of the dependency_snapshot.dependencies.select statement.

@brrygrdn brrygrdn force-pushed the brrygrdn/extract-refresh-version-update-operation branch from 99adc98 to e98c3f4 Compare March 31, 2023 11:00
@brrygrdn brrygrdn force-pushed the brrygrdn/extract-refresh-version-update-operation branch from e98c3f4 to 24a7a9c Compare March 31, 2023 12:12
@brrygrdn brrygrdn merged commit 6017b09 into main Mar 31, 2023
@brrygrdn brrygrdn deleted the brrygrdn/extract-refresh-version-update-operation branch March 31, 2023 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants