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

bot: Single Approval Paths #206

Merged
merged 6 commits into from
Dec 29, 2023
Merged

bot: Single Approval Paths #206

merged 6 commits into from
Dec 29, 2023

Conversation

jimbishopp
Copy link
Contributor

@jimbishopp jimbishopp commented Dec 26, 2023

This PR addresses the "Single Approver" issue described in #205.

Add the ability for certain PRs to require a single approval.

A new setting has been added, singleApproverPaths, that defines paths in a repository that only require a single approver. If all the files in a PR only match paths defined in singleApproverPaths, then the PR only requires a single approval. Otherwise, the PR requires the default number of approvals, which is currently defined as 2.

A second setting was added, singleApproverAuthors, that defines PR authors that only require a single approval (dependency bots in cloud for now).

Add the ability for certain PRs to require a single approval.

A new setting has been added to the bot configuration file,
singleApproverPaths, that defines paths in a repository that
only require a single approver.

If all the files in a PR only match paths defined in
singleApproverPaths, then the PR only requires a single approval.
Otherwise, the PR requires the default number of approvals, which is
currently defined as 2.
@jimbishopp jimbishopp requested review from a team December 26, 2023 23:49
bot/internal/bot/bot.go Outdated Show resolved Hide resolved
@jimbishopp
Copy link
Contributor Author

jimbishopp commented Dec 27, 2023

I added a commit that moves the single approver paths from the config file to a Go literal.

@jimbishopp
Copy link
Contributor Author

jimbishopp commented Dec 28, 2023

I added another commit that allows certain authors such as dependabot or renovate to only require a single approver..

bot/internal/bot/bot.go Outdated Show resolved Hide resolved
// a set of authors that only require a single approver. 1 is returned when all of the
// files match a single approver path or the PR author matches one of the authors, otherwise
// env.DefaultApproverCount is returned.
func approverCount(authors, paths []string, author string, files []github.PullRequestFile) int {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we enforce some sort of naming convention for bots that only require a single approval as an extra precaution? I don't think we'd ever want an employee to unconditionally only require a single approval.

Alternatively, we can just require that any author configured as a single approval also passes isAllowedRobot.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. I added a test to ensure only bots can be configured as single approver authors. We can also export isAllowedRobot from the review package and test in approverCount if you think we should have a secondary set of checks.

bot/internal/bot/bot.go Outdated Show resolved Hide resolved
@jimbishopp jimbishopp merged commit 70967df into main Dec 29, 2023
8 checks passed
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.

3 participants