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

feat: auto merge dependabot PRs for patch updates #188

Closed
wants to merge 2 commits into from

Conversation

Mossaka
Copy link
Member

@Mossaka Mossaka commented Jul 19, 2023

This PR adds a new action called "Dependabot automation" which will automatically merge dependabot PRs for patch updates. According to this doc,

If you enable auto-merge for a pull request, the pull request will merge automatically when all required reviews are met and all required status checks have passed.

which means the PRs won't be auto-merged if the tests gate fails or it doesn't have the required number of reviews.

Hence I've also added gh pr review --approve "$PR_URL to automatically approve the dependabot PRs.

Since this automates the process of merging dependabot PRs, I've set the schedule interval for depdnabot to "daily".

Close #150

Could you please take a look? @utam0k

@Mossaka Mossaka requested review from cpuguy83 and utam0k July 19, 2023 23:14
@Mossaka Mossaka closed this Jul 19, 2023
@Mossaka Mossaka reopened this Jul 19, 2023
.github/dependabot.yml Outdated Show resolved Hide resolved
Signed-off-by: jiaxiao zhou <jiazho@microsoft.com>
@jprendes
Copy link
Collaborator

which means the PRs won't be auto-merged if the tests gate fails or it doesn't have the required number of reviews.

Does this mean that any failing CI test would prevent the auto merge?
IIUC, the main branch should be protected from failing CI tests. Is this currently the case?

@cpuguy83
Copy link
Member

Is this currently the case

Just reviewed branch protection.
It is now.
PR's also require an approver.

Honestly I'm not particularly on board with auto-merging dependabot, especially knowing our CI is missing some important checks (like validating protos).

@Mossaka
Copy link
Member Author

Mossaka commented Jul 21, 2023

@cpuguy83 thanks for the suggestion. I am going to create an issue for adding more CI checks like validaitng protos.

It seems like this PR won't be merged in until we've done the above first. I will convert it to a draft and leave it there.

@Mossaka Mossaka marked this pull request as draft July 21, 2023 17:56
@Mossaka
Copy link
Member Author

Mossaka commented Jul 24, 2023

@cpuguy83 it looks like we already are validaitng protos in the CI according to this comment. Do you think you will be okay with merging this PR now?

Signed-off-by: Jiaxiao Zhou <jiazho@microsoft.com>
@cpuguy83
Copy link
Member

I still don't like auto-merging dependabot, but I wouldn't want to block it if others find it useful.

@Mossaka
Copy link
Member Author

Mossaka commented Aug 16, 2023

@cpuguy83 would you please state the reasons why you don't like auto-merging dependantabot? This will help me understand the tradeoffs better.

@jsturtevant
Copy link
Contributor

I am not a big fan of auto-merging because it doesn't give awareness to maintainers. I like the manual step reviewing and understanding what changed even if it takes an extra step. Even if tests pass there might be a reason not to merge (either something like missing tests or some other external reason (one example could be a library gets compromised, and we need it pinned to a particular version).

In summary, I love having dependa-bot do the manually labor but like the human check before merging.

@Mossaka
Copy link
Member Author

Mossaka commented Aug 22, 2023

Closing this PR

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.

Patch version updates automated by Dependabot
5 participants