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 process for requesting fast review on pull requests? #204

Closed
domenic opened this issue Feb 18, 2021 · 3 comments
Closed

A process for requesting fast review on pull requests? #204

domenic opened this issue Feb 18, 2021 · 3 comments

Comments

@domenic
Copy link
Member

domenic commented Feb 18, 2021

@chrishtr and I were brainstorming ways of helping the WHATWG be more efficient. The issue triage meeting idea in whatwg/html#6371 is one direction. Another we were thinking of is helping pull requests land faster.

When things go well, we land pull requests really quickly. I'll mention some recent examples from Fetch (intentionally not pinging folks with @):

  • annevk's refactoring work, with me as reviewer
  • annevk's work on fragment preservation, with wanderview and davidben as reviewers.
  • yutakahirano's recent work on WebTransport integration and service worker interception, with annevk and jakearchibald as reviewers.

The common threads in such cases are: the change has general support, reviewers respond really quickly (usually <12 hours), and the PR author responds to reviews really quickly (also usually <12 hours).

However, there are other cases where despite the change having general support, it takes a long time to get the PR landed due to reviewer or PR author lag. I don't really want to call out specific instances for fear of calling out specific reviewers or PR authors, but I expect people have seen this happen and have their own instances in mind.

The proposal would be to have some way for a PR author to request "fast review" for a pull request. I'd envision this mostly being used for something which has general agreement, but seems to have stalled for bandwidth reasons. The PR author makes a commitment to respond to all reviews within their current or early-next working day, and asks reviewers to try to do the same. Then, all parties involved decide to prioritize such fast-review PRs and help them land sooner rather than later.

To be clear, taking a while to get around to one's reviews is not a personal failing! Life happens; we all have lots of responsibilities. None of this would be binding; nobody's going to kick you out of the WHATWG if you take too long to review a pull request. I'm more trying to create a coordination point, where we collectively agree "yes, let's all prioritize this review highest". And it would help avoid negative feedback loops, like "oh, last time I left review comments the PR author took a day to respond, so I can probably wait a couple of days before re-reviewing".

What do folks think?

@annevk
Copy link
Member

annevk commented Feb 18, 2021

I like the idea of both sides making time to get a thing done, as that is usually more efficient than having to page it in weeks later (though sometimes such delays pay off in unexpected ways).

One thing I've been thinking about is that sometimes more context would help. Some kind of summary that guides you through the changes, without which you basically have to start from scratch ensuring that it all holds together, which can take quite a bit of time. Whether this is needed varies, but for complicated changes this could reduce the time spent by the reviewer quite a bit and as such the author might be motivated to provide it so the turnaround is quicker. (Other signs I sometimes look for are tests, have browser bugs already been filed, etc.)

@foolip
Copy link
Member

foolip commented Feb 18, 2021

I like this idea! Would there be some kind of handshake whereby the parties agree to prioritize the reviews? Would editors be expected to monitor these PRs closely if they're not the reviewer and poke people if needed?

@domenic
Copy link
Member Author

domenic commented Feb 18, 2021

Yeah, those both sounds like what I'd be envisioning. I'm not sure how formal to make the "handshake"...

I like the idea of encouraging such PRs to have more context and up-front work done (like tests and browser bugs). If a reviewer is going to prioritize it, the PR author should make that as easy as possible for them.

@domenic domenic closed this as completed Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants