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

Automatically approve dependabot PRs #704

Closed
ItsDrike opened this issue Dec 15, 2023 · 5 comments · Fixed by #714
Closed

Automatically approve dependabot PRs #704

ItsDrike opened this issue Dec 15, 2023 · 5 comments · Fixed by #714

Comments

@ItsDrike
Copy link
Member

          > I am a bit overwhelmed by dependabot already, but why not

This frequency can be reduced. Those PRs are a low priority as most do not affect the code that mcstatus is leveraging. If there's a bot that could automatically approve and merge dependabot updates that pass CI tests, that would be amazing.

Originally posted by @kevinkjt2000 in #697 (comment)

@PerchunPak
Copy link
Member

There is a setting somewhere in repository settings which allows this. But I am okay if we run dependabot once a month or so, anyway, it has almost zero effect on us.

@ItsDrike
Copy link
Member Author

If we only run once a month, once that time hits, we'd see potentially a dozen new PRs come up all at once, which I'd say is equally as annoying (especially since dependabot will do a rebase after every merge that also modifies pyproject.toml/poetry.lock). Having a bot that auto-approves would be a better choice IMO, but yeah, as a temporary solution we can bump up the time period.

@PerchunPak
Copy link
Member

We could also switch to renovate, it can group dependencies into one PR.

@ItsDrike
Copy link
Member Author

I'm not sure I love the idea of groupping the updates, while it does mean a lot less review work and completely fixes the issue with having to wait for auto-rebasing after every merge, there is an issue with this approach.

With a single PR per single dependency update, we can very easily see exactly which dependencies caused a CI failure on updating. This often happens with pyright, as they make a lot of changes which modify the type-checking behavior, potentially introducing new checks. Similarly, it can happen with ruff linter. But it can also happen when a runtime dependency makes a breaking change for example.

it's pretty nice to have these CI failures separate, as it immediately shows us which package update caused this issue, whereas with a big group update, there may be a whole bunch of issues from multiple dependencies, which we would then need to debug and track down.

I'm not necessarily completely against doing this, but I do feel like this is a pretty nice feature of per-dependency PRs. This would be especially nice if we had auto-approvals, as the updates which failed the CI wouldn't get merged, even if approved (CI passing is required for merging), so we would only see those updates which failed and require manual fixes.

@PerchunPak
Copy link
Member

We could easily move pyright and ruff to their groups, and keep them separate from other dependencies.

https://docs.renovatebot.com/noise-reduction/#package-grouping

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 a pull request may close this issue.

2 participants