Conversation
This change adds the CODEOWNERS config that ensure that a review is required from the software team for any changes being made to the codebase.
There was a problem hiding this comment.
Pull Request Overview
This PR adds a CODEOWNERS file to enforce code review requirements. The goal is to require software team approval for all codebase changes, while allowing the genomic-analysis-team to approve changes to the scripts folder independently. However, the current implementation has a pattern ordering issue that prevents it from working as intended.
- Introduces CODEOWNERS file with two ownership rules
- Sets software-team as default owners for all files
- Attempts to set genomic-analysis-team as owners for scripts folder (but pattern ordering prevents this from working correctly)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * @populationgenomics/software-team | ||
| /scripts/ @populationgenomics/genomic-analysis-team @populationgenomics/software-team |
There was a problem hiding this comment.
The order of patterns in CODEOWNERS is incorrect. In CODEOWNERS files, more specific patterns must be placed before more general patterns, otherwise they won't override the default.
Currently, line 1 (*) matches everything including /scripts/, so line 2's pattern is redundant. To achieve the stated goal where the genomic-analysis-team can approve changes in /scripts/ without requiring software-team approval, you should:
- Move the
/scripts/pattern to line 1 - Move the
*pattern to line 2
Corrected order:
/scripts/ @populationgenomics/genomic-analysis-team
* @populationgenomics/software-team
This way, scripts folder changes only require genomic-analysis-team approval, while all other files require software-team approval.
| * @populationgenomics/software-team | |
| /scripts/ @populationgenomics/genomic-analysis-team @populationgenomics/software-team | |
| /scripts/ @populationgenomics/genomic-analysis-team | |
| * @populationgenomics/software-team |
There was a problem hiding this comment.
I'm pretty sure this is wrong, the documentation here:
https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners
states that "Order is important; the last matching pattern takes the most precedence."
2 participants
This change adds the CODEOWNERS config that ensure that a review is required from the software team for any changes being made to the codebase but allows the other teams to make changes to the
scriptsfolder.