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

Ideas to reduce the PR backlog #253

Open
jodh-intel opened this issue Mar 2, 2022 · 4 comments
Open

Ideas to reduce the PR backlog #253

jodh-intel opened this issue Mar 2, 2022 · 4 comments
Labels
enhancement Improvement to an existing feature needs-help Request for extra help (technical, resource, etc)

Comments

@jodh-intel
Copy link
Contributor

Background

Review rota

The Kata community has a very small team of volunteer reviewers who give up some of their time to review the PR and issue backlogs.

Why are reviewers important?

The reviewers are the gatekeepers of the Kata project. They help to ensure the correctness, consistency, legibility, maintainability and general overall quality of the contents of the Kata repositories.

Do we really need reviewers?

Yes, they are absolutely 100% completely entirely totally categorically omg essential! Kata would not be what it is today without them. See above!

What do you need to be a reviewer?

Anyone can be a reviewer, whether you are on the rota or not. See and then feel free to start comments on PRs!:

Problem

We have too many open PRs.

Hence, more specifically the problem can be defined as:

  • PRs are taking too long to land.
  • We don't have enough volunteers on the review rota.
  • We need more "knowledge transfer" from those with specialist knowledge to help others be able to review PRs in more detail.

Remember

Every open PR is:

  • An improvement or new feature you cannot use yet.
  • A bugfix you cannot benefit from yet.

In other words PRs are only directly useful to the community of users once they have been merged. But they cannot be merged until they have been reviewed and approved.

Solutions

Can you help?

If you have spare time, please help us by reviewing any open PRs. It's great that we have so many PRs being raised, but every PR raised means more work for the review team.

Reminders:

  • You do not need to be an expert to help review any PR - all comments are potentially useful so please feel free to ask away!
  • We need to keep every PR moving by "nudging" it with comments, suggestions and approvals.

Review documentation

We have lots of helpful documentation for reviewers:

The following documents are also potentially useful for reviewers to read:

Proposals

"Raise 1, review 2"

If you raise a PR, please consider reviewing atleast two other PRs.

Present at the AC meeting

If you have specialist knowledge that could be shared with the community, please consider presenting at the Architecture Committee meeting. This will help others to help you as others will be able to help review PRs "in your area".

AC meeting plan

As discussed in the AC meeting on 2022-03-01, we're going to try expanding the scope of our CODEOWNERS files to focus reviews towards the teams that can best review particular PRs.

Plan

  • Update the CODEOWNERS files.
  • Review one month after they have landed to see if they are helping or not.
@jodh-intel jodh-intel added enhancement Improvement to an existing feature needs-help Request for extra help (technical, resource, etc) labels Mar 2, 2022
jodh-intel added a commit to jodh-intel/tests-1 that referenced this issue Mar 2, 2022
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 added a commit to jodh-intel/kata-containers that referenced this issue Mar 2, 2022
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#3804.

Signed-off-by: James O. D. Hunt <[email protected]>
@jodh-intel
Copy link
Contributor Author

RFC PRs raised - please comment! 😄

@wainersm
Copy link
Contributor

wainersm commented Mar 4, 2022

Hi @jodh-intel , thanks for raising those issues and proposing solutions.

I came from email based communities (e.g. QEMU) where it is a common practice to have a MAINTAINERS file to keep the inventory of maintainers and reviewers. And there are tools to copy patch series emails to the maintainers and reviewers too. I am still a heavy user of emails, and I have filters to organize my inbox; so github notifications where I was mentioned will land to my inbox and get my attention. I (unfortunately) don't have the habit of scanning github pull requests. So I'm saying all this to emphasize that I like the idea of the codeowners files being up-to-date as much as possible; that will help me to create filters and/or habits of reviewing PRs for the areas I am interested and is more knowledgeable.

@wainersm
Copy link
Contributor

wainersm commented Mar 4, 2022

Hi @jodh-intel , thanks for raising those issues and proposing solutions.

I came from email based communities (e.g. QEMU) where it is a common practice to have a MAINTAINERS file to keep the inventory of maintainers and reviewers. And there are tools to copy patch series emails to the maintainers and reviewers too. I am still a heavy user of emails, and I have filters to organize my inbox; so github notifications where I was mentioned will land to my inbox and get my attention. I (unfortunately) don't have the habit of scanning github pull requests. So I'm saying all this to emphasize that I like the idea of the codeowners files being up-to-date as much as possible; that will help me to create filters and/or habits of reviewing PRs for the areas I am interested and is more knowledgeable.

Just to make it more clear: very often I loose the opportunity to review important PRs because I was not mentioned nor not marked as reviewer, then the github notifications goes to /dev/null... :(

@jodh-intel
Copy link
Contributor Author

Thanks @wainersm. If you and others could tal at the proposed changes for the codeowners file, that would be great:

jodh-intel added a commit to jodh-intel/kata-containers that referenced this issue Nov 16, 2023
Improve the `CODEOWNERS` file by specifying more groups.

Since GitHub automatically checks the `CODEOWNERS` file when a PR is
created and adds all matching groups as reviewers for the PR, this may
help reduce the PR backlog since the right people will be alerted and
requested to review the PR. That should improve the quality of reviews
(and thus the quality of the landed code). It may also have a positive
effect on PR velocity.

> **Note:**
>
> This PR combines the other `CODEOWNERS` files so we have
> a single, visible, top-level file.

See: kata-containers/community#253

Fixes: kata-containers#3804.

Signed-off-by: James O. D. Hunt <[email protected]>
sprt pushed a commit to sprt/kata-containers that referenced this issue Dec 1, 2023
Improve the `CODEOWNERS` file by specifying more groups.

Since GitHub automatically checks the `CODEOWNERS` file when a PR is
created and adds all matching groups as reviewers for the PR, this may
help reduce the PR backlog since the right people will be alerted and
requested to review the PR. That should improve the quality of reviews
(and thus the quality of the landed code). It may also have a positive
effect on PR velocity.

> **Note:**
>
> This PR combines the other `CODEOWNERS` files so we have
> a single, visible, top-level file.

See: kata-containers/community#253

Fixes: kata-containers#3804.

Signed-off-by: James O. D. Hunt <[email protected]>
sprt pushed a commit to microsoft/kata-containers that referenced this issue Dec 7, 2023
Improve the `CODEOWNERS` file by specifying more groups.

Since GitHub automatically checks the `CODEOWNERS` file when a PR is
created and adds all matching groups as reviewers for the PR, this may
help reduce the PR backlog since the right people will be alerted and
requested to review the PR. That should improve the quality of reviews
(and thus the quality of the landed code). It may also have a positive
effect on PR velocity.

> **Note:**
>
> This PR combines the other `CODEOWNERS` files so we have
> a single, visible, top-level file.

See: kata-containers/community#253

Fixes: kata-containers#3804.

Signed-off-by: James O. D. Hunt <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement to an existing feature needs-help Request for extra help (technical, resource, etc)
Projects
None yet
Development

No branches or pull requests

2 participants