Title
r
Ricardo Delfin
05/25/2023, 1:18 PMHello! I've got an interesting bug that has to do with merging. This PR (private repo, not sure if the link is useful) has been approved by all owners and can be merged from GitHub without any issues. However, graphite claims I can't merge it because there are missing code owners. Any idea why that might be? Thanks!
I've had to merge this PR but this seems to be happening on all my PRs today
j
Jacob Gold
05/25/2023, 3:02 PM@Brendan Ngo , can you take a look?
b
Brendan Ngo
05/25/2023, 3:12 PMI'll take a look!
hm looks like you've merged the pr already so we've lost that state
we've started calculating required codeowners on Graphite which is why you're seeing that message now. If you see this again can you let me know? It may have been a problem with us missing some data on our side at the time. On our side now it does look like the pr has all owners.
also question, how was reviewing on Graphite with codeowners required before? we didn't respect it at all so I'm sure our mergeability calculations were never right for you?
r
Ricardo Delfin
05/26/2023, 1:30 PMOh! Really sorry. Yeah, I'll ping you next time I see it happen
Before, it just didn't let me merge I thought, though I might have missed it letting me. I usually monitor who's approved before I press merge
That said... We also have a bot that tells you the minimum set of reviewers you need for a PR (codeowners get... Complicated real fast, and our codeowners file is quite large and uses some weird github behaviour related to how they evaluate codeowners)
b
Brendan Ngo
05/26/2023, 4:15 PMbefore, if you had minimum number of approvals you could try to merge on graphite but if we were missing required codeowners it would fail doing the merge, which is the reason we wanted to start enforcing codeowners on the graphite side. codeowners does get complicated quickly, especially for prs with lots of files with lots of different codeowners. Hopefully our new codeowner checks work for you but definitely let us know if you see something weird again!
r
Ricardo Delfin
05/30/2023, 11:19 AMOk, I've got another PR
That I can merge in github but not graphite
b
Brendan Ngo
05/30/2023, 5:05 PMOh I see the problem now! Looks like our codeowners calculation is right but we don't have data for those teams so we couldn't link the reviewers to the team codeowners. We should have this data so looking into why it's missing!
There's a fix going out now that should fix this going forward! Thanks for flagging and let me know if you see anything unexpected still
r
Ricardo Delfin
05/31/2023, 8:32 AMOooh that's interesting! Is it just that you don't have access to our owners groups int the integration? But thanks for looking into this!
b
Brendan Ngo
05/31/2023, 3:16 PMwe should have permission there was just a bug in our teams logic. codeowners is also tricky since there isn't an api for it so we need to rederive everything ourselves