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

[Fix] multi-directory grouped updates sometimes create ungrouped PRs #9938

Merged
merged 37 commits into from
Jun 20, 2024

Conversation

Nishnha
Copy link
Member

@Nishnha Nishnha commented Jun 7, 2024

What are you trying to accomplish?

Depends on #9988
This PR is stacked on top of #9988

Multi-directory grouped updates sometimes create ungrouped PRs because the #run_ungrouped_dependency_updates method relied on some old assumptions that have changed.

#run_ungrouped_dependency_updates makes use of DependencySnapshot#ungrouped_dependencies to check which dependencies do not belong to a group and require an individual PR.
This used to work, but we recently changed the way we calculate if a dependency belongs to a group with the introduction of multi-directory updates.

When Dependabot could only update 1 directory at a time, marking a dependency as "handled" with DependencySnapshot#add_handled_dependencies meant that it had been processed.

When we added multi-directory updates, we kept track of handled_dependencies per directory in case we handle a dependency in one directory and don't skip over it in a later directory.

This meant that when we changed the current_directory in the DirectorySnapshot, we lost the list of handled_dependencies, so dependencies belonging to an existing pull request group are not considered handled and are included in the ungrouped_dependencies list.

To account for this, this PR introduces DependencySnapshot#add_handled_group_dependencies and DependencySnapshot#handled_group_dependencies to mark every dependency in a group as handled across all directories and retrieve them.

Anything you want to highlight for special attention from reviewers?

This work depends on a few other PRs which also share the :dependency_has_directory feature flag. These other PRs add in a directory field to Dependabot::Dependency that this PR uses to track which dependencies are handled.

How will you know you've accomplished your goal?

We have a silent ecosystem test in this PR. This is also feature flagged so we can test it out on our test repo.

Checklist

  • I have run the complete test suite to ensure all tests and linters pass.
  • I have thoroughly tested my code changes to ensure they work as expected, including adding additional tests for new functionality.
  • I have written clear and descriptive commit messages.
  • I have provided a detailed description of the changes in the pull request, including the problem it addresses, how it fixes the problem, and any relevant details about the implementation.
  • I have ensured that the code is well-documented and easy to understand.

@Nishnha
Copy link
Member Author

Nishnha commented Jun 13, 2024

I think we can still use the specs and silent ecosystem tests from #9610 in this

@Nishnha Nishnha changed the base branch from main to nishnha/propagate-directory June 19, 2024 20:03
@github-actions github-actions bot added L: ruby:bundler RubyGems via bundler L: python labels Jun 19, 2024
@Nishnha Nishnha force-pushed the nishnha/handled-dependency-group branch from e208228 to fd5f82e Compare June 20, 2024 07:12
@Nishnha Nishnha force-pushed the nishnha/propagate-directory branch from 98aabc0 to 2edad0e Compare June 20, 2024 07:12
@Nishnha Nishnha force-pushed the nishnha/handled-dependency-group branch from 30f0644 to 1afe4e6 Compare June 20, 2024 08:13
@Nishnha Nishnha marked this pull request as ready for review June 20, 2024 13:54
@Nishnha Nishnha requested a review from a team as a code owner June 20, 2024 13:54
@Nishnha Nishnha changed the base branch from nishnha/propagate-directory to main June 20, 2024 15:32
@Nishnha Nishnha marked this pull request as draft June 20, 2024 16:10
@Nishnha Nishnha changed the base branch from main to nishnha/propagate-directory June 20, 2024 16:11
@Nishnha Nishnha force-pushed the nishnha/handled-dependency-group branch 2 times, most recently from e3a9685 to db8b2d3 Compare June 20, 2024 16:14
@Nishnha Nishnha force-pushed the nishnha/handled-dependency-group branch from d2a939a to 88d39c7 Compare June 20, 2024 16:48
@Nishnha Nishnha force-pushed the nishnha/handled-dependency-group branch from fedfbeb to 766eab7 Compare June 20, 2024 18:11
@Nishnha Nishnha force-pushed the nishnha/propagate-directory branch from 8a16bc8 to 0a3e581 Compare June 20, 2024 19:26
@Nishnha Nishnha force-pushed the nishnha/handled-dependency-group branch from 766eab7 to 88d39c7 Compare June 20, 2024 19:28
@Nishnha Nishnha force-pushed the nishnha/propagate-directory branch from 39600d2 to 8962ae7 Compare June 20, 2024 19:48
@Nishnha Nishnha force-pushed the nishnha/propagate-directory branch from 8962ae7 to dfe9c96 Compare June 20, 2024 19:54
Base automatically changed from nishnha/propagate-directory to main June 20, 2024 20:55
@Nishnha Nishnha merged commit 9307016 into main Jun 20, 2024
121 checks passed
@Nishnha Nishnha deleted the nishnha/handled-dependency-group branch June 20, 2024 21:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
L: python L: ruby:bundler RubyGems via bundler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants