WIP: Add CODEOWNERS #4551
WIP: Add CODEOWNERS #4551
Conversation
|
QA Wolf here! As you write new code it's important that your test coverage is keeping up. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
pierremtb
requested review from
jessfraz,
nadr0,
jtran,
lf94,
adamchalmers,
franknoirot,
Irev-Dev and
max-mrgrsk
With this change, we're all going to get requested as a reviewer on every PR. Is that intended? I don’t personally like this change because the approval requirement isn’t meant to block merges. It’s meant to help spread knowledge and get more eyes on code. If someone makes a PR and someone on the outside approves, could people simply not merge? |
|
@jtran Good point, the noise part would for sure be annoying. Would work better with a @kittycad/frontend team. Closing for now. |
|
Created @KittyCAD/frontend as a nested team under @KittyCAD/eng. So probably worth reopening this discussion? (low priority of course) |
|
@pierremtb the bot will delete that group unless you add it here https://github.com/KittyCAD/configs/blob/main/configs/groups.toml copying the settings for consultants should be fine, that way its internal only |
|
@jessfraz ah thanks for the heads up, will PR there |
|
I'm seeing an error message when I look at the code change:
|
|
Yeah me too @franknoirot. I did create https://github.com/KittyCAD/configs/pull/132 to at least populate that team and then we can come back here and see if we need anything else? |
Following an outside approval on #4516, it seems like this is something we should start having?
See https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners
The team @KittyCAD/eng already has write access, and seems like a straightforward owner here. Could restrict to frontend at some point but I don't think it's needed now.
This coupled with this setting below turned on in the branch protection section should ensure only KittyCAD engineers approvals count toward the PR merge criterion.
