-
Notifications
You must be signed in to change notification settings - Fork 19
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
chore: update codeowners #1004
chore: update codeowners #1004
Conversation
CODEOWNERS
Outdated
@@ -1,4 +1,4 @@ | |||
# https://help.github.com/articles/about-codeowners/ | |||
# https://git-scm.com/docs/gitignore#_pattern_format | |||
|
|||
* @grafana/synthetic-monitoring-dev | |||
* @ckbedwell @w1kman @VikaCep |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to limit the set of people who can approve PRs, it would be better to have a different team, e.g. synthetic-monitoring-fe
or synthetic-monitoring-frontend
as that is easier to manage (you make changes to the team instead of having to make changes to this file).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't want to limit who can approve them, I want to ensure there is accountability with who is assigned. I don't like teams being assigned as reviewers because it often ends up the same as no-one being assigned as a reviewer. I'm happy to have a team as the code owner but I'd want to change how PRs are assigned as reviewers. Can we change it so that GitHub does some sort of randomised round robin where it assigns two people specifically from the new team? (In our case it wouldn't make a difference but would help when we add new people to the sm-fe-team).
Another reason I want this is because of how it plays with the github-reminders Slack channel. I want it so reviewers are explicitly tagged with reminders so they know they have a review to perform rather than going unnoticed because of it just writing the team string and not tagging anybody.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I'm following correctly, this should do the trick.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does require that you change the CODEOWNERS from @grafana/synthetic-monitoring-dev
to @grafana/synthetic-monitoring-fe
(feel free to suggest a different name).
Also, if that's what you want, that PR needs an approval (use atlantis apply
to merge it, do not merge it yourself).
I agree that manage this via teams is probably the best way to maintain this. Assuming that individuals from that team gets assigned (accountability). With that said, the team is small enough for us to start with individuals as code owners and bite the management bullet when we get there. |
Problem
@grafana/synthetic-monitoring-dev currently gets automatically assigned as a reviewer for this repository.
Solution
The above made sense when the team was much smaller but as the team is now significantly bigger with three engineers effectively owning this repository it makes sense to update the owners to be reflective of that and remove the manual step of assigning the frontend engineers in SM as reviewers.