search

NCAR / ccpp-physics

GFS physics for CCPP
Other
59 stars 146 forks source link

CODEOWNERS not automatically being added as code reviewers #835

Closed grantfirl closed 2 years ago

grantfirl commented 2 years ago

Description

For https://github.com/NCAR/ccpp-physics/pull/832, the gwdps.f was modified, but the POC listed in the CODEOWNERS file was not automatically added as a reviewer. @grantfirl added the GitHub username of the listed POC as a reviewer manually.

Steps to Reproduce

Change a file in ccpp-physics. Submit a PR and see if GitHub automatically adds the GitHub user listed in CODEOWNERS to the reviewer list.

@SamuelTrahanNOAA I thought this was working at one time. I'm not sure if anything changed to break this functionality or if it didn't work right from the beginning?

SamuelTrahanNOAA commented 2 years ago

Has the CODEOWNERS failed for other PRs recently?

grantfirl commented 2 years ago

I haven't been paying very much attention to this, but Ligia asked and I promised to check. The first one that I was paying attention to failed. As another example, it looks like #823 didn't automatically add Hannah Barnes and Haiqin Li who are codeowners for cu_gf*.

SamuelTrahanNOAA commented 2 years ago

I'm going to do a test PR that changes each file individually and see who is invited.

SamuelTrahanNOAA commented 2 years ago

I have submitted a ticket with to the NCAR helpdesk to contact their github engineers via their support contract. I suspect there is a syntax error in the CODEOWNERS file that is causing github to ignore the entire file.

ligiabernardet commented 2 years ago

Sam, a Google search indicates that there are some codes "out there" to validate CODEOWNERS files. Pls take a look and consider if that would be helpful.

SamuelTrahanNOAA commented 2 years ago

@grantfirl I changed repository settings, and your bug should now be fixed. Please confirm.

My test is here: #861 which successfully requested reviews from everyone.

One note though: the main CODEOWNERS points to mdtoyNOAA, who is not a member, instead of mdtoy, who is. For my test in #861, I changed the test-codeowners-2 branch CODEOWNERS to mdtoy to make sure that user is invited to review.

grantfirl commented 2 years ago

Fixed