-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
propose additional CODEOWNERS #313
Conversation
Please try to follow the rules of who is eligible to become an approver: If all people are eligible try to prove the requirements. |
Deferring to @rghetia and @jmacd who wanted to have additional help with approving Go PRs, even if people don't necessarily meet the count and duration criteria. I created this PR at their request. additionally, I've been informed @freeformz no longer is able to participate, so removing that nomination. |
Please update the PR accordingly with the description or vice versa :) |
Did you forget to push the changes? |
I did indeed, I typed |
I'll discuss this issue at Governance Committee today - whether maintainers can override org-wide policies about minimum approver code/review counts while we're still bootstrapping... |
@lizthegrey I don't think otel-go is in bootstrapping phase anymore, we are getting close to have a working implementation and a defined API. |
…y-go into lizf.codeowners
Let me put it this way: @sjkaris and @pjanotti's activity has dropped off in this specific repo (but they have been active elsewhere), so we're down to just 2 maintainers, plus one active approver here. everyone else who's an ordinary contributor but not approver, has unfortunately, less than 3 months of continuous active contribution, so I feel like we need to relax the criteria in order to get more current active contributors as approvers to keep things moving forward and spread the load out for poor @jmacd, @rghetia, and @tedsuo. Especially if we're worried about people from one company dominating approvals (since 2/3 are from Lightstep) |
Hi all – this PR/topic came up again at the OTel GC meeting. I volunteered to "be the bad guy" on this one, so here goes... Basically, everyone is sympathetic to the particulars of this situation (especially the unsustainable load that had been placed on the the maintainers, which I understand was the primary motivator here). That said, especially as the OTel project continues to embiggen, it is important that we strive for consistency – both out of fairness, and also to prevent potential bad actors in the future from citing "exceptions" in the past to justify things that might not be so innocent. There are basically two options here:
Personally, I'm partial to "change the rules", as it is rough to have a small number of maintainers in a SIG with lots of activity. Per these guidelines, that would mean changing the "maintainer" criteria for the Go SIG. If we don't want to change the rules, though, the OTel GC asks that the Go SIG rolls this back until the formal criteria for "maintainer" have been satisfied. |
In this case it's changing the rules for Approver not for Maintainer, but I agree we should change the rules. |
I apologize. We didn't intend to skirt the rules, but the feeling was that there were several members of the community who were very involved and had demonstrated good judgement. It felt like we had not enough "official" approvers given the number of qualified approvers. |
(@lizthegrey @jmacd: sorry for my delay) Yeah, I understand and think that makes intuitive sense... to be clear, the overall message here is not meant to be as much a "request-for-apology" (the intentions here seemed good/pure), but more a "request-for-rules-change." Can one of you two take care of that with the Go SIG this coming week? Thank you. |
To be clear, is this a request for us to circle back to the Go SIG to get collective agreement to go back to Governance asking for the rules change? I feel like I flagged this in Governance Committee a month ago when we were first about to do this, and thus it feels the request for rules change has already been made ;) |
We've had a couple of approvers remove themselves, and I think @tedsuo
should do that as well.
As a result, we have quota for more approvers.
…On Sun, Dec 8, 2019 at 9:54 PM Liz Fong-Jones ***@***.***> wrote:
To be clear, is this a request for us to circle back to the Go SIG to get
collective agreement to go back to Governance asking for the rules change?
I feel like I flagged this in Governance Committee a month ago when we
were first about to do this, and thus it feels the request for rules change
has already been made ;)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#313?email_source=notifications&email_token=AA3WFCP57NYA47FY2JTPTMTQXXMQNA5CNFSM4JNEHOW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGH5UGI#issuecomment-563075609>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA3WFCKVTAFDKOQNI2F4HBDQXXMQNANCNFSM4JNEHOWQ>
.
|
The main issue I think here is not that we shouldn't have added more approvers, the issue is the "3 continuous months / 30 pulls reviewed & approved" criterion for becoming an approver that we skirted when we added the new round. |
The rules don't say what we should do when the initial set of approvers (who did not meet the criterion) remove themselves. I still don't really understand the github permissions model (e.g., https://github.com/orgs/open-telemetry/teams/go-approvers/members says we have 6 approvers if you count the two maintainers). I'll just stay out of governance issues. |
Okay, I'll drive the governance rules change then by proposing that: (1) we have emeritus or otherwise remove inactive approvers, (2) if the # of active approvers drops below a certain level, the the approver minimum criteria can be relaxed subject to maintainer's discretion... stand by :) |
Rules were officially changed, so now all the people added in this pull meet the requirements. |
- [x] >10 substantive contributions - [x] Active >1mo - [x] add to `CODEOWNERS` (done already in #313) - [x] Maintainer nomination (done already in #313) - [ ] Add to @open-telemetry/go-approvers - [x] Add to `CONTRIBUTING.md` (done in this pull)
As per @rghetia and @jmacd's desire to add current active contributors to the approvers list:
Propose adding @iredelmeier @paivagustavo @krnowak @lizthegrey as additional CODEOWNERS.