chore: put collective ownership on most CODEOWNERS #2788
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2788 +/- ##
==========================================
- Coverage 60.83% 60.82% -0.02%
==========================================
Files 563 563
Lines 75169 75169
==========================================
- Hits 45730 45720 -10
- Misses 26072 26077 +5
- Partials 3367 3372 +5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Just to set expectations upfront: @gnolang/devrels are not codeowners of the docs since some time back - check out this comment. However, I don't mind providing inputs. |
leohhhn
reviewed
Sep 12, 2024
moul
requested changes
Sep 12, 2024
|
I would suggest adding |
Kouteki
reviewed
Sep 13, 2024
| # - Directories and files which are owned by one specific user X are to be | ||
| # interpreted as "primarily maintained by X". Any significant changes to | ||
| # that given code should be reviewed by X; as if it were another repository | ||
| # where X is the primary developer. However, ownership here can be ignored if |
There was a problem hiding this comment.
Should we suggest to the individual code owner to unassign themselves if the change is small enough?
There was a problem hiding this comment.
They can, but it's labour, and I'm not sure github won't reassign them anyway.
…/gno into dev/morgan/codeowners-but-simple
|
This PR will be pending until @aeddi figures out a bot for github which will have a wider scope, and cover some of the things the CODEOWNERS file was currently attempting to do :) |
|
Looking to actually remove CODEOWNERS |
11 tasks
This was referenced Nov 6, 2024
Closed
thehowl
pushed a commit
that referenced
this pull request
Nov 27, 2024
This pull request aims to add a bot that extends GitHub's functionalities like codeowners file and other merge protection mechanisms. Interaction with the bot is done via a comment. You can test it on the demo repo here : GnoCheckBot/demo#1 Fixes #1007 Related to #1466, #2788 - The `config.go` file contains all the conditions and requirements in an 'If - Then' format. ```go // Automatic check { Description: "Changes to 'tm2' folder should be reviewed/authored by at least one member of both EU and US teams", If: c.And( c.FileChanged(gh, "tm2"), c.BaseBranch("main"), ), Then: r.And( r.Or( r.ReviewByTeamMembers(gh, "eu", 1), r.AuthorInTeam(gh, "eu"), ), r.Or( r.ReviewByTeamMembers(gh, "us", 1), r.AuthorInTeam(gh, "us"), ), ), } ``` - There are two types of checks: some are automatic and managed by the bot (like the one above), while others are manual and need to be verified by a specific org team member (like the one below). If no team is specified, anyone with comment editing permission can check it. ```go // Manual check { Description: "The documentation is accurate and relevant", If: c.FileChanged(gh, `.*\.md`), Teams: []string{ "tech-staff", "devrels", }, }, ``` - The conditions (If) allow checking, among other things, who the author is, who is assigned, what labels are applied, the modified files, etc. The list is available in the `condition` folder. - The requirements (Then) allow, among other things, assigning a member, verifying that a review is done by a specific user, applying a label, etc. (List in `requirement` folder). - A PR Check (the icon at the bottom with all the CI checks) will remain orange/pending until all checks are validated, after which it will turn green. <img width="1065" alt="Screenshot 2024-11-05 at 18 37 34" src="https://github.com/user-attachments/assets/efaa1657-c254-4fc1-b6d1-49c7b93d8cda"> - The Github Actions workflow associated with the bot ensures that PRs are processed concurrently, while ensuring that the same PR is not processed by two runners at the same time. - We can manually process a PR by launching the workflow directly from the [GitHub Actions interface](https://github.com/GnoCheckBot/demo/actions/workflows/bot.yml). <img width="313" alt="Screenshot 2024-11-06 at 01 36 42" src="https://github.com/user-attachments/assets/287915cd-a50e-47a6-8ea1-c31383014b84"> #### To do - [x] implement base version of the bot - [x] cleanup code / comments - [x] setup a demo repo - [x] add debug printing on dry run - [x] add some tests on requirements and conditions <!-- please provide a detailed description of the changes made in this pull request. --> <details><summary>Contributors' checklist...</summary> - [x] Added new tests, or not needed, or not feasible - [x] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [x] Updated the official documentation or not needed - [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [x] Added references to related issues and PRs - [x] Provided any useful hints for running manual tests </details>
r3v4s
pushed a commit
to gnoswap-labs/gno
that referenced
this pull request
Dec 10, 2024
…#3037) This pull request aims to add a bot that extends GitHub's functionalities like codeowners file and other merge protection mechanisms. Interaction with the bot is done via a comment. You can test it on the demo repo here : GnoCheckBot/demo#1 Fixes gnolang#1007 Related to gnolang#1466, gnolang#2788 - The `config.go` file contains all the conditions and requirements in an 'If - Then' format. ```go // Automatic check { Description: "Changes to 'tm2' folder should be reviewed/authored by at least one member of both EU and US teams", If: c.And( c.FileChanged(gh, "tm2"), c.BaseBranch("main"), ), Then: r.And( r.Or( r.ReviewByTeamMembers(gh, "eu", 1), r.AuthorInTeam(gh, "eu"), ), r.Or( r.ReviewByTeamMembers(gh, "us", 1), r.AuthorInTeam(gh, "us"), ), ), } ``` - There are two types of checks: some are automatic and managed by the bot (like the one above), while others are manual and need to be verified by a specific org team member (like the one below). If no team is specified, anyone with comment editing permission can check it. ```go // Manual check { Description: "The documentation is accurate and relevant", If: c.FileChanged(gh, `.*\.md`), Teams: []string{ "tech-staff", "devrels", }, }, ``` - The conditions (If) allow checking, among other things, who the author is, who is assigned, what labels are applied, the modified files, etc. The list is available in the `condition` folder. - The requirements (Then) allow, among other things, assigning a member, verifying that a review is done by a specific user, applying a label, etc. (List in `requirement` folder). - A PR Check (the icon at the bottom with all the CI checks) will remain orange/pending until all checks are validated, after which it will turn green. <img width="1065" alt="Screenshot 2024-11-05 at 18 37 34" src="https://github.com/user-attachments/assets/efaa1657-c254-4fc1-b6d1-49c7b93d8cda"> - The Github Actions workflow associated with the bot ensures that PRs are processed concurrently, while ensuring that the same PR is not processed by two runners at the same time. - We can manually process a PR by launching the workflow directly from the [GitHub Actions interface](https://github.com/GnoCheckBot/demo/actions/workflows/bot.yml). <img width="313" alt="Screenshot 2024-11-06 at 01 36 42" src="https://github.com/user-attachments/assets/287915cd-a50e-47a6-8ea1-c31383014b84"> #### To do - [x] implement base version of the bot - [x] cleanup code / comments - [x] setup a demo repo - [x] add debug printing on dry run - [x] add some tests on requirements and conditions <!-- please provide a detailed description of the changes made in this pull request. --> <details><summary>Contributors' checklist...</summary> - [x] Added new tests, or not needed, or not feasible - [x] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [x] Updated the official documentation or not needed - [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [x] Added references to related issues and PRs - [x] Provided any useful hints for running manual tests </details>
albttx
pushed a commit
that referenced
this pull request
Jan 10, 2025
This pull request aims to add a bot that extends GitHub's functionalities like codeowners file and other merge protection mechanisms. Interaction with the bot is done via a comment. You can test it on the demo repo here : GnoCheckBot/demo#1 Fixes #1007 Related to #1466, #2788 - The `config.go` file contains all the conditions and requirements in an 'If - Then' format. ```go // Automatic check { Description: "Changes to 'tm2' folder should be reviewed/authored by at least one member of both EU and US teams", If: c.And( c.FileChanged(gh, "tm2"), c.BaseBranch("main"), ), Then: r.And( r.Or( r.ReviewByTeamMembers(gh, "eu", 1), r.AuthorInTeam(gh, "eu"), ), r.Or( r.ReviewByTeamMembers(gh, "us", 1), r.AuthorInTeam(gh, "us"), ), ), } ``` - There are two types of checks: some are automatic and managed by the bot (like the one above), while others are manual and need to be verified by a specific org team member (like the one below). If no team is specified, anyone with comment editing permission can check it. ```go // Manual check { Description: "The documentation is accurate and relevant", If: c.FileChanged(gh, `.*\.md`), Teams: []string{ "tech-staff", "devrels", }, }, ``` - The conditions (If) allow checking, among other things, who the author is, who is assigned, what labels are applied, the modified files, etc. The list is available in the `condition` folder. - The requirements (Then) allow, among other things, assigning a member, verifying that a review is done by a specific user, applying a label, etc. (List in `requirement` folder). - A PR Check (the icon at the bottom with all the CI checks) will remain orange/pending until all checks are validated, after which it will turn green. <img width="1065" alt="Screenshot 2024-11-05 at 18 37 34" src="https://github.com/user-attachments/assets/efaa1657-c254-4fc1-b6d1-49c7b93d8cda"> - The Github Actions workflow associated with the bot ensures that PRs are processed concurrently, while ensuring that the same PR is not processed by two runners at the same time. - We can manually process a PR by launching the workflow directly from the [GitHub Actions interface](https://github.com/GnoCheckBot/demo/actions/workflows/bot.yml). <img width="313" alt="Screenshot 2024-11-06 at 01 36 42" src="https://github.com/user-attachments/assets/287915cd-a50e-47a6-8ea1-c31383014b84"> #### To do - [x] implement base version of the bot - [x] cleanup code / comments - [x] setup a demo repo - [x] add debug printing on dry run - [x] add some tests on requirements and conditions <!-- please provide a detailed description of the changes made in this pull request. --> <details><summary>Contributors' checklist...</summary> - [x] Added new tests, or not needed, or not feasible - [x] Provided an example (e.g. screenshot) to aid review or the PR is self-explanatory - [x] Updated the official documentation or not needed - [x] No breaking changes were made, or a `BREAKING CHANGE: xxx` message was included in the description - [x] Added references to related issues and PRs - [x] Provided any useful hints for running manual tests </details>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR comes following a discussion in today's "review meeting", where we talked about our current approach to ownership.
One aspect which was talked about was that the current set-up for CODEOWNERS is very "noisy". Opening up a PR often entails tagging many people and teams for review. To try to overcome this, I removed most rules which tried to find "exact people" to own a piece of the code, in favour of collective ownership on the
tech-staff,security,devrelsanddevopsteams, depending on which part of the code is interested.I wrote a few lines explaining the differences between assigning a directory to a team, a person, or a team + person, and how the CODEOWNERS file should be interpreted. I wrote them trying to match the practical implications of the CODEOWNERS file (= who gets picked in the requested reviewers) with the "social" / organizational implications.