Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Grouped Updates] Cleaner management of the update dependency list #7414

Merged
merged 5 commits into from Jun 16, 2023

Conversation

brrygrdn
Copy link
Contributor

@brrygrdn brrygrdn commented Jun 8, 2023

This PR moves the management of the overall Dependency list into the GroupDependencyFileBatch class and repositions it as a DependencyGroupChangeBatch.

This affords us an interface where we just pass in each DependencyChange and the right thing happens internally to the DependencyGroupChangeBatch for both the files and dependency list without the actual group updater worrying about the details.

De-duplication

I've had a walk through the implications of us updating a Dependency twice in a single group, this has the potential to happen more for transitive dependencies if we do something like:

  • dep_a and dep_b both depend on dep_x
  • We update dep_a which unlocks updating dep_x to the highest version currently permitted by dep_b
  • We update dep_b which now unlocks updating dep_x to a newer version

This means we would have two Dependency objects for dep_x with would have the following attributes:

  1. name: dep_x, version: 1.4, previous_version: 1.3
  2. name: dep_x, version: 1.5, previous_version: 1.4

If we were to de-duplicate this, we would need to replace these two Dependency objects with a new one which has the following values:

  • name: dep_x, version: 1.5, previous_version: 1.3

This is pretty non-trivial as the Dependency class mutates some of it's input arguments so we cannot instantiate replacement from the two objects and be confident it is correct, nor can we just merge the previous_version of (1) into (2) as we might not correctly capture the metadata for the entire leap from v1.3 to v1.4.

Ultimately, I've decided to leave this alone but give us some debug affordance so we can investigate the dependency list and learn more about these cases if and when they happen in future.

Potential Improvements

Rather than mutating data and risking creating unforeseen corner cases, this is mostly a presentation concern when it comes to us creating the Pull Request body.

Overall, this is likely to be an edge case and if in the worst case we show two bumps for a library in the PR body with a changelog from 1.3 to 1.4, and then from 1.4 to 1.5 the information presented is still complete end-to-end and represents what happened within the update accurately.

If we do decide to deduplicate as a presentation concern, we will not need to mutate the dependency data but we can use it to render a single aggregate entry as a follow-up which feels less risky.

I haven't explored this as part of this PR as:

  • It means we need to modify the PullRequest builder to handle dependencies-grouped-by-name instead of individual dependencies in several places which is non-trivial
  • We have some other work in the pipe to change the PR content for groups I don't want to collide with on such a significant change
  • I'm not convinced this will happen frequently enough that the default behaviour of showing two bumps to the dependency isn't sufficient

@brrygrdn brrygrdn marked this pull request as ready for review June 13, 2023 14:02
@brrygrdn brrygrdn requested a review from a team as a code owner June 13, 2023 14:02
@brrygrdn brrygrdn self-assigned this Jun 13, 2023
@brrygrdn brrygrdn force-pushed the brrygrdn/cleaner-management-of-updated-dependencies branch from 502ea79 to cf6b71d Compare June 16, 2023 11:38
@brrygrdn brrygrdn merged commit c7a600f into main Jun 16, 2023
83 checks passed
@brrygrdn brrygrdn deleted the brrygrdn/cleaner-management-of-updated-dependencies branch June 16, 2023 12:45
brettfo pushed a commit to brettfo/dependabot-core that referenced this pull request Oct 11, 2023
…anagement-of-updated-dependencies

[Grouped Updates] Cleaner management of the update dependency list
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.

None yet

2 participants