Skip to content
This repository has been archived by the owner on Jun 28, 2024. It is now read-only.

codeowners: Expand scope #4534

Closed

Conversation

jodh-intel
Copy link
Contributor

Improve the CODEOWNERS file by specifying more groups. The hope is
this will help reduce the PR backlog.

See: kata-containers/community#253

Fixes: #4533.

Signed-off-by: James O. D. Hunt [email protected]

Improve the `CODEOWNERS` file by specifying more groups. The hope is
this will help reduce the PR backlog.

See: kata-containers/community#253

Fixes: kata-containers#4533.

Signed-off-by: James O. D. Hunt <[email protected]>
@jodh-intel jodh-intel added rfc Requires input from the team do-not-merge PR has problems or depends on another labels Mar 2, 2022
@jodh-intel
Copy link
Contributor Author

As shown, the CODEOWNERS file contains errors relating to non existant teams. If folk agree, I'll create those...

@wainersm
Copy link
Contributor

wainersm commented Mar 7, 2022

Hi @jodh-intel !

What's your opinion about having individuals on the file? For example, assigning myself to .ci/openshift-ci instead of the broaden ci team.

Also maybe the directory kata-webhook should be owned by the integration-tests team as I is used by some of the k8s integration tests.

@katacontainersbot katacontainersbot added the size/small Small and simple task label Mar 9, 2022
@jodh-intel
Copy link
Contributor Author

@wainersm - personally, I'm not in favour of hard-coding usernames in these files since it would create a maintenance nightmare as folks come and go, files move, etc. By only specifying teams, the file will be smaller and clearer. Plus, we can manage group membership without requiring PRs to make it more agile.

@wainersm
Copy link
Contributor

@wainersm - personally, I'm not in favour of hard-coding usernames in these files since it would create a maintenance nightmare as folks come and go, files move, etc. By only specifying teams, the file will be smaller and clearer. Plus, we can manage group membership without requiring PRs to make it more agile.

You got a point. And it makes sense for me. :)

@wainersm
Copy link
Contributor

Is it in wip because some groups need to be created?

@jodh-intel
Copy link
Contributor Author

@wainersm - for that reason and also that I'd like some input from the community on the content of the PR :)

@wainersm
Copy link
Contributor

@wainersm - for that reason and also that I'd like some input from the community on the content of the PR :)

Got it.

/cc @kata-containers/redhat please your input is valuable...

@c3d
Copy link
Member

c3d commented Mar 23, 2022

@jodh-intel I like the idea, but we need to create the associated groups.

It's unclear to me why you believe that maintaining the groups is easier than maintaining the code owners file. Is it because you expect contributors to self-managed and exit the groups by themselves? Or some other reason?

@jodh-intel
Copy link
Contributor Author

@jodh-intel I like the idea, but we need to create the associated groups.

@c3d - yep, see my comment above. I've held off creating those groups until there is general agreement to the strategy.

My thoughts on specifying groups rather than individual users:

Convenience / easier maintenance

The codeowners file lives in the repo, which means we have to raise a PR every time we adjust it.

By specifying groups rather than users we can maintain the members using GH's existing teams interface.

Clarity

If we just specify the groups, the file is clearer (simpler and shorter). But also imagine a rule like this:

*.wibble        @user1, @user2, @user3

That doesn't tell me much if I'm new to the project (wtf are wibble files and who are those users?) Conversely:

*.wibble        @kata-containers/security-team

Although I still don't know what wibble files are, I can take a guess as to what they are used for and/or their relative importance, so the end result is more (but admittedly not complete) clarity.

Ease of contact

The reason for creating this file is to simplify the task of the PR reviewers. If a reviewer wants input from a group of contributors relating to, say, a security PR, it's much easier for them to ping a single GitHub group than potentially ping a long list of users.

Privacy

There is the possibility that some users may not wish to be "known" as being involved in certain areas of the code. If we specify groups, they can maintain some level of privacy since you need a certain GitHub privilege level to be able to view group members.

Personal preference

  • I don't like unnecessary hard coding.
  • Lists of users:
    • Look untidy to me.
    • Are invariably incorrect / out of date.

Hence, I'd personally prefer a level of indirection to hide the clutter and make the end result neater 😄

@jodh-intel
Copy link
Contributor Author

Closing due to lack of feedback...

@jodh-intel jodh-intel closed this Apr 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
do-not-merge PR has problems or depends on another rfc Requires input from the team size/small Small and simple task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update CODEOWNERS file
4 participants