Skip to content
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

Merged
merged 5 commits into from
Nov 14, 2019
Merged

propose additional CODEOWNERS #313

merged 5 commits into from
Nov 14, 2019

Conversation

lizthegrey
Copy link
Member

@lizthegrey lizthegrey commented Nov 14, 2019

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.

@bogdandrutu
Copy link
Member

Please try to follow the rules of who is eligible to become an approver:
https://github.com/open-telemetry/community/blob/master/community-membership.md#approver

If all people are eligible try to prove the requirements.

@lizthegrey
Copy link
Member Author

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.

@bogdandrutu
Copy link
Member

Please update the PR accordingly with the description or vice versa :)

@rghetia
Copy link
Contributor

rghetia commented Nov 14, 2019

additionally, I've been informed @freeformz no longer is able to participate, so removing that nomination.

Did you forget to push the changes?

@lizthegrey
Copy link
Member Author

additionally, I've been informed @freeformz no longer is able to participate, so removing that nomination.

Did you forget to push the changes?

I did indeed, I typed git push but then forgot to tap my security key haha

@lizthegrey
Copy link
Member Author

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...

CODEOWNERS Outdated Show resolved Hide resolved
@bogdandrutu
Copy link
Member

@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.

@lizthegrey
Copy link
Member Author

lizthegrey commented Nov 14, 2019

@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.

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)

@rghetia rghetia merged commit f403198 into open-telemetry:master Nov 14, 2019
@lizthegrey lizthegrey deleted the lizf.codeowners branch November 14, 2019 17:29
@bhs
Copy link

bhs commented Dec 6, 2019

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:

  1. Roll back the change here
  2. Change the rules

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.

@lizthegrey
Copy link
Member Author

In this case it's changing the rules for Approver not for Maintainer, but I agree we should change the rules.

@jmacd
Copy link
Contributor

jmacd commented Dec 6, 2019

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.

@bhs
Copy link

bhs commented Dec 9, 2019

(@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.

@lizthegrey
Copy link
Member Author

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 ;)

@jmacd
Copy link
Contributor

jmacd commented Dec 10, 2019 via email

@lizthegrey
Copy link
Member Author

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.

@jmacd
Copy link
Contributor

jmacd commented Dec 10, 2019

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.

@lizthegrey
Copy link
Member Author

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 :)

@lizthegrey
Copy link
Member Author

@lizthegrey
Copy link
Member Author

Rules were officially changed, so now all the people added in this pull meet the requirements.

lizthegrey added a commit that referenced this pull request Jan 24, 2020
- [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)
@pellared pellared added this to the untracked milestone Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants