🌱 Define fine-grained owners for the various subcomponents of OLMv1. #2113
Conversation
The goal is to reduce the possibility of the bystander effect (see https://en.wikipedia.org/wiki/Bystander_effect), and give maintainers more accountability and ownership of the areas in which they are experts. This will also help contributors more quickly identify those experts and get the necessary reviews for their work to merge.
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2113 +/- ##
=======================================
Coverage 73.60% 73.60%
=======================================
Files 78 78
Lines 7260 7260
=======================================
Hits 5344 5344
Misses 1566 1566
Partials 350 350
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
I'm good with this. I'll leave the final approvals after all the folks you mentioned in the description have had a say. |
Signed-off-by: Joe Lanford <[email protected]>
| docs-draft-approvers: | ||
| - camilamacedo86 | ||
| - grokspawn | ||
| - thetechnick |
There was a problem hiding this comment.
I'm super-unclear on the aggregation/override model here, but if michaelryanpeter is not approver in docs-draft-approvers, IMO he should be.
There was a problem hiding this comment.
The structure is <root>/docs/draft. Because of OWNERS inheritance, the following OWNERS files are scraped/unioned for approvers/reviewers:
<root>/OWNERS<root>/docs/OWNERS<root>/docs/draft/OWNERS
Michael is an approver in (2), which makes him an approver in (3) via inheritance.
I raised one point, but I think this is a fine trial approach. We can adjust in future where there is friction, or to meet specific support goals. |
|
I am okay with trying it out and seeing how that works. |
| @@ -0,0 +1,2 @@ | |||
| approvers: | |||
| - api-approvers | |||
There was a problem hiding this comment.
Changes to the api directory often lead to changes in the config directory, which leads to changes in the manifest directory. We might want to consolidate these a bit.
There was a problem hiding this comment.
Added OWNERS with api-approvers for the generated CRD manifests. Good catch!
Signed-off-by: Joe Lanford <[email protected]>
|
+1 lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tmshort The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
(Took off LGTM, otherwise, this might merge unintentionally) |
|
/lgtm |
openshift-merge-bot
bot
merged commit 0206ad4
into
operator-framework:main
6 participants
The goal is to reduce the possibility of the bystander effect (see https://en.wikipedia.org/wiki/Bystander_effect), and give maintainers more accountability and ownership of the areas in which they are experts.
This will also help contributors more quickly identify those experts and get the necessary reviews for their work to merge.
Description
In order to help me come up with the groupings and assignments, I assessed git commit history and a sampling of GitHub PRs (using the
ghcommand, don't worry I didn't manually trawl PRs) to find trends in contributions and reviews. I feel like what that showed lined up with what I somewhat expected just based on my past observations.We won't merge this PR without approvals from all of the existing approvers:
@perdasilva @tmshort @grokspawn @camilamacedo86 @thetechnick
Reviewer Checklist