Not sure we’ve ever looked into or discussed the idea of adding the code owners file, so here we go
The code owners file allows to automatically assign developers that own specified code/file/directory when a PR is open that modifies those files/directories. Draft PRs do not automatically assign reviewers.
Here is a proposal: add .gitea/CODEOWNERS and allow people to opt-in to claim ownership. I wouldn’t “mandate” to have full list of owners or try to match it with the module owners, but I’d really love to be assigned as a reviewer for areas like Libmv, depsgraph, maybe even Cycles. It’ll simplify the process of keeping an eye on an open PRs that I might be very interested in. Maybe it is something other developers would like to do as well?
I was not aware of such feature, and… I like it a lot!
I’d also love to be automatically kept up to date with PRs affecting several core areas (ID management, RNA, BLO, bpy…). Not necessarily to actually review all of it, but at least be aware of what’s happening in these areas.
It could be quite helpful, especially for new contributors to find reviewers. There could be some issues to work out for (large) code cleanups or unclear ownership. But might as well try with a few developers to see how it goes, and then if it works well try to map almost the whole code to owners.
This is maybe being somewhat ‘semantic’, but anyway, my very very first thought was that the term ‘owner’ for an open source project felt a little business/commercial like.
Maybe, Maintainer or Custodian or Guardian or something more along a FOSS line rather then I wrote this code, I’m the Owner, its my IP, etc, etc.
Anyway, just a thought and yes, Happy New Year, at least it is for me.
I’ve committed the current state of the PR as a “test drive”. Will see how well it works etc.
Meanwhile, if someone wants to participate in the experiment please feel free to add self to the owners file
Ok, first experiment results: it broke PR creation with the following error: Error 500: <!DOCTYPE html> <html lang="en-US" data-theme="bthree-dark"> <head> <meta name="viewport" content=
The change is reverted for now. Will test it on staging to see whats going on.
The error seemed to not be related to the CODEOWNERS, but to a stale lock files. Somehow the issue coincided with us landing the change (which seemed to cause the issue) and reverting it back (which seemed to solve the problem).
The CODEOWNERS file is now live, with fixed syntax (Gitea expects actual regex, not wildcard), and we’ve quickly tested that PRs against Cycles are automatically assigning reviewers.
Let me know if there are any issues which you believe are related to this file!
It make some sense to keep the reviewers list up to date as the PR gets updated. But it’s something to be aware of if you see unexpected changes to reviewers.
No reviewers will be added if you create a pull request with WIP in the title, to prevent people getting notified too early.
While objectively an improvement this does expose an issue we had since the phab migration:
How do i tell the difference between “I must have this reviewers blessing before landing” vs “this person just subscribed to stay informed and i can safely land without this person explicitly signing off” ?
In the phab days we had the concept of a “blocking reviewer” to differentiate between these two rather distinct options, while things are a lot less clear now.
People should not see CODEOWNERS as a “subscription” to the PR. If someone adds themselves to the file they are responsible for the review, or removing self from the reviewers if they think it is not up to them to review.
Yes, there is nothing in Gitea preventing that. It just adds reviewers on every update.
Another problem is that on the timeline there is no indication that reviewers were added automatically by CODEOWNERS, it looks the same as manually adding a reviewer.
This feature could certainly use some polish, but not sure where it ranks in priorities.
Another problem is that on the timeline there is no indication that reviewers were added automatically by CODEOWNERS, it looks the same as manually adding a reviewer.
If that PR introduces a new “reserved username”, we may need to update our BlenderID-to-Gitea bridge as well, to prevent errors when somebody has gitea-codeowners (or whatever they settle on) as username in Blender ID.
Edit: Lunny writes “I don’t think we need a system bot user here”, so hopefully this won’t be necessary.
