search

exercism / website-copy

A repository for exercism's website's copy
203 stars 950 forks source link

codeowners: is this file actually considerd by github to assign reviewers? #1249

Open NobbZ opened 5 years ago

NobbZ commented 5 years ago

Today I had to manually assign reviewers to a PR, so I checked if that entry is missing in the CODEOWNERS file. Its not. Therefore I checked more PRs:

I'm not sure now if #1248 was just a hickup of GitHub not having assigned the team, and the other 3 were properly assigned but assignment not visible in the logs, or if none of the PRs was actually correctly assigned for review.

Perhaps someone can provide more datapoints, as I'm out of time for now.

NobbZ commented 5 years ago

After further investigation, I found that #1188 had a review requested, as some logmessage said "approved these changes on behalf of exercism/website-copy-rust".

The "on behalf of" is missing on all comments, requests and/or approval in #1215 and #1217.

So either something happened inbetween #1188 and #1215 that caused codeowners to not work anymore, or there is some other reason for my observations.

PS: the codeowners file hasn't been touched between those PRs, as its untouched for 5 months now…

iHiD commented 5 years ago

I've assigned @kytrinyx to this as she has both the Exercism magics and the GitHub magics :)

kytrinyx commented 5 years ago

My first thought is that CODEOWNERS probably won't assign a team, even if the team is in the file, if the team doesn't have the correct permissions on the reop (I've been bit by that at work a couple of times). I don't know if the team needs write access, or if read access is enough. I will find out.

iHiD commented 5 years ago

Great spot. Docs say:

People with admin or owner permissions can set up a CODEOWNERS file in a repository. The people you choose as code owners must have write permissions for the repository.

I've set the teams to have write permission to see if it helps. @Nobbz - could you report back as to whether its made a difference pls?

kytrinyx commented 5 years ago

I've also confirmed with the team that owns that feature at GitHub, as well.

NobbZ commented 5 years ago

could you report back as to whether its made a difference pls?

I'll keep an eye on it over the next couple of days.

NobbZ commented 5 years ago

Needed to assign reviewers manually in #1267