Skip to content
This repository has been archived by the owner on Aug 2, 2021. It is now read-only.

remove GitHub codeowners #1089

Closed
wants to merge 1 commit into from
Closed

Conversation

nonsense
Copy link
Contributor

@nonsense nonsense commented Jan 3, 2019

For many packages we have only one code owner and their approval is a must when merging a PR. What happens when these people go on vacation for example? (see ethereum/go-ethereum#18386) - we won’t be able to merge anything in swarm/api/http when @justelad is on vacation.

We must add at least 2-3 code owners to every single package, or get rid of them altogether.

I don’t really like adding many code owners, because then you have a PR with 4-5 reviewers, and noone feels responsible for reviewing the PR.

Therefore I suggest we remove all the codeowners from .github. What do you think?

I think we all have a good idea of who knows which part of the codebase best, and can make a good educated guess who to add as a reviewer, without the need for codeowners.

@nonsense
Copy link
Contributor Author

nonsense commented Jan 3, 2019

If this gets 2 more approvals, I will submit to ethereum/go-ethereum, otherwise let's quickly discuss on roundtable.

@nonsense nonsense requested a review from skylenet January 3, 2019 15:33
@frncmx
Copy link
Contributor

frncmx commented Jan 4, 2019

You just sold this PR to me by being blocked if 1 person on vacation. That's a no go.

On the other hand I think - even with the approval - the topic might worth a discussion.

  • I don't have the confidence about picking reviewers based on their code familiarity. I used a different heuristic; I usually request a review from somebody who is currently working on something related. => That might not be good, if we have "owners".
  • Also the volume of ownership fragmentation, i.e., number of lines in the ownership files, seems a little concerning to me. => Is Swarm/the team that big? Is Swarm/the team big enough to have some kind of ownership rules (less areas) or shall we expect everybody to be familiar with the code?
  • I think having owners and automatically assigned reviewers might be helpful for first time contributors, e.g., 1 timers, before ("homework") and after freshly hired.
  • Having explicit ownership might be also good, because we might avoid silos, i.e., just one person knowing a specific part. Personally, if I have my name on something I tend to care more, so I would try to keep an eye what is changing in "my" part of the code base.

I let it up to you @nonsense or @zelig to decide if we shall talk about ownership.

@zelig
Copy link
Member

zelig commented Jan 9, 2019

merged as ethereum/go-ethereum#18412

@zelig zelig closed this Jan 9, 2019
@frncmx frncmx deleted the remove-github-owners-swarm branch January 10, 2019 18:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants