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

A modest proposal to fix our pull request latency. #8573

Open
burtonator opened this issue Jul 23, 2024 · 0 comments
Open

A modest proposal to fix our pull request latency. #8573

burtonator opened this issue Jul 23, 2024 · 0 comments
Assignees
Labels
2 Half day task bug Something isn't working

Comments

@burtonator
Copy link
Collaborator

burtonator commented Jul 23, 2024

Describe the bug

I had an idea about how we could encourage devs to review PRs more so we can have higher PR throughput and keep the PR size lower.

I think this is important to reduce the burden of getting massive PRs reviewed.

I think as a team we all agree that we want to make it easy to have our PRs reviewed quickly but I think we're sort of stuck in a catch-22 where:

  1. we have a lot of PRs
  2. this increases PR review latency
  3. which encourages us to create larger PRs which are harder to review
  4. which slows PR review throughput
  5. ... therefore, goto number 1

What if we blocked our PRs via a Github action that would fail if we have too many PRs (or older ones).

We would create a new Github action that looks at:

  • Number of open PRs
  • Age of the oldest open PR

I think we can get this data just by doing a 'curl' and getting the JSON response for our open project PRs.

If any of these exceed a threshold we fail CI for that PR (we could ignore draft PRs)

The number of open PRs would be k * nr_developers. Everyone on the team would be allowed 3 open PRs but it would be somewhat flexible since we multiply it by the number of developers on the entire team.

We're at 50 PRs now... some of these are drafts. Some of these are old and should probably be closed.

If we block merging PRs that don't pass CI this would force developers to go back to reviewing PRs before they can get their PR merged.

I still think we're falling behind on PR reviews and it's causing PR size to increase and also merge conflicts to increase too which is just slowing us down.

I have a larger PR that is going to be difficult to merge just because it's spiraling out of control. I've noticed that for the Knock changes this has been somewhat problematic due to touching a large amount of code.

Curious what you think here... I'm just asking a few people on the team to get their feedback.

We could just accept comments for now and then maybe discuss by voice later this week.

Initial conditions

Environment:

Branch/Release version:

Browser:

Wallet:

Reproduction steps

Actual behavior

Expected behavior

Screenshots / Video

Reporter

Additional context

@burtonator burtonator added bug Something isn't working 2 Half day task labels Jul 23, 2024
@burtonator burtonator self-assigned this Jul 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 Half day task bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant