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 edge cases during semver grouping creating single PRs erroneously #7867

Merged
merged 10 commits into from
Aug 23, 2023

Conversation

jakecoffman
Copy link
Member

This pushes the "handled" logic into the group so that the compile_updates_for method can specify that the dependency is handled immediately when it is found to be in the semver grouping. This way, if there is an error or the update is not possible, it will still be considered handled and not attempt to create a single PR for it.

Also, if an exception is thrown, even during version fetching before we can know whether it's in the grouping or not, we consider the dependency handled as well since it's not known what versions are available. Customers can request a rebase or recreate to try again and not have superfluous PRs kicking about.

@jakecoffman jakecoffman force-pushed the jakecoffman/handle-ungrouped-semver-issues branch from e35cfb9 to 5ee63e8 Compare August 22, 2023 14:09
@jakecoffman
Copy link
Member Author

jakecoffman commented Aug 22, 2023

The specs have shown an issue and now I'm seeing both groups and dependency_snapshots have dependencies. It seems like the snapshot has the dependencies that are already filtered on by patterns, and the group dependencies are populated as the update proceeds? A little unclear...

Wait, no the dependency engine populates the group dependencies... !?

Edit 74: After studying the code, I think the failing specs are misleading. Now that we're in the world where ungrouped_dependencies is only populated by actually doing an update, they only make sense if add_to_handle is called. So I updated the test.

@jakecoffman
Copy link
Member Author

I need to add a test and then this will be ready to go.

@jakecoffman jakecoffman marked this pull request as ready for review August 23, 2023 13:13
@jakecoffman jakecoffman requested a review from a team as a code owner August 23, 2023 13:13
@jakecoffman jakecoffman enabled auto-merge (squash) August 23, 2023 16:00
@jakecoffman jakecoffman merged commit 1e17026 into main Aug 23, 2023
90 checks passed
@jakecoffman jakecoffman deleted the jakecoffman/handle-ungrouped-semver-issues branch August 23, 2023 16:21
brettfo pushed a commit to brettfo/dependabot-core that referenced this pull request Oct 11, 2023
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