Requre a third pair of eyes in Kayobe reviews #429
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @cityofships, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request aims to enhance code quality and reduce post-merge issues for Kayobe-related repositories by increasing the required number of approving reviews for pull requests. This change applies to several Kayobe branch protection rules, ensuring a more thorough review process before code is merged.
Highlights
- Enhanced Code Review Requirements: The primary change increases the required_approving_review_count from 1 to 2 for pull requests targeting branches protected by Kayobe-related rules.
- Scope of Change: This modification affects branch protection rules for caracal, epoxy, and master branches within the Kayobe category, as defined in terraform/github/branches.tf.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request increases the required number of approving reviews from one to two for the caracal, epoxy, and master branches in Kayobe repositories. This is a sensible change to improve code quality by requiring more oversight on important branches. The implementation is correct. I've added one comment regarding code duplication that could be addressed in the future to improve maintainability. Also, there's a small typo in the pull request title ('Requre' should be 'Require').
| required_pull_request_reviews { | ||
| dismiss_stale_reviews = true | ||
| require_code_owner_reviews = true | ||
| required_approving_review_count = 1 | ||
| required_approving_review_count = 2 | ||
| } |
There was a problem hiding this comment.
This required_pull_request_reviews block, along with most of the surrounding resource block, is duplicated for the caracal, epoxy, and master branch protection rules. This pattern of duplication is present throughout the file for other branch protection rules as well.
While a full refactoring is likely out of scope for this PR, consider consolidating these resources in the future. Using a single github_branch_protection resource with a for_each meta-argument over a map of branch configurations would significantly reduce code duplication. This would make the rules easier to manage and update consistently.
|
Great in theory, but review velocity is already dreadful as it is. Nothing is going to make progress if every PR needs two reviews. |
Terraform Format and Style 🖌
|
|
Sounds good but I also think we need to bring this in Infra meeting first. |
Proposing this change so it might help in catching more issues before and not after the changes are merged. The main target being s-k-c repository, it currently shares protection rules with 'Kayobe' category repos. Could create a separate dedicated rule if we wanted.