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

META - Review Team #2890

Open
2 tasks
thehowl opened this issue Oct 3, 2024 · 6 comments
Open
2 tasks

META - Review Team #2890

thehowl opened this issue Oct 3, 2024 · 6 comments
Assignees

Comments

@thehowl
Copy link
Member

thehowl commented Oct 3, 2024

@thehowl thehowl self-assigned this Oct 3, 2024
@Kouteki
Copy link
Contributor

Kouteki commented Oct 3, 2024

Notes

  • The review team should focus on PRs only, they are currently a bottleneck.
  • The review team should focus on contributor PRs, and ignore PRs made by the core team. Over time, and as contributors get more familiar with the project, we'll revisit this decision.
  • Failing CI is a valid reason to reject a PR.
  • Guideline content: if a PR is intentionally failing CI, the guideline should state that the author needs to state that it's intentional.
  • Guideline content: PRs should reference the issues they are addressing
  • Guideline content: dependabot PR guidelines
  • The review team should feel free to review PRs from their teams (i.e. be the 1st review). The 2nd review should be the core team.

Action items

  • @Kouteki to tag all issues that the review team should look at
  • @Kouteki to notify the review team once the labels are added, so they could start reviewing
  • @Kouteki to research possible improvements to the New issue templates

@jefft0
Copy link
Contributor

jefft0 commented Oct 17, 2024

A related issue uses the term "facilitators", which I like. We are using the label "review team" which is confusing. (One contributor thought that removing the "review team" label means that the PR won't be reviewed.) Can we rename the "review team" label to "facilitators"?

@Kouteki
Copy link
Contributor

Kouteki commented Oct 17, 2024

Notes from the Oct 17 meeting:

  • The review team's goal shouldn't be to address the huge PR backlog; that's on the core team. Instead, they should help us with the top of the funnel, i.e. new PRs from external contributors that will only increase in volume over time.
  • Right now we have multiple PRs trying to tackle the review flow: Setup "nice" stale PRs' bot #1445 Proposal: improving review process with bot-assisted checkboxes #1007 chore: add code facilitators ci #1660
  • We'll add the PR review flow on the core team agenda next week
  • In the short term, I've changed the label 'review team' to 'review/triage-pending' and updated the label description
  • Once we have a bot up, we'll automate the flow so that the review team can be assigned automatically on external PRs
  • Our best idea right now is to get rid off CODEOWNERS, don't assign anyone automatically.
  • We'll use the Signal chat for internal coordination, and this meta issue for note keeping and planning.

@Kouteki Kouteki self-assigned this Oct 17, 2024
@Kouteki
Copy link
Contributor

Kouteki commented Oct 17, 2024

Optimum review flow

  • Pick a PR with 'review/triage-pending'
  • Assign yourself & review a PR
  • Approve or request changes
  • Remove the 'review/triage-pending' label

@Kouteki
Copy link
Contributor

Kouteki commented Oct 26, 2024

A couple of updates:

  • chore: disable CODEOWNERS #3022 merged
  • Dependabot should be treated as an external contributor. Review Dependabot PRs if you feel comfortable, they will be tagged for this team.

@Kouteki
Copy link
Contributor

Kouteki commented Nov 6, 2024

Reviewers have a triage role. This means that they:

  • Can open/close issues & PRs
  • Can review PRs
  • Can't review conversations on other people's PRs

@thehowl thehowl changed the title META - Review Team Phase 1 META - Review Team Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Triage
Development

No branches or pull requests

3 participants