Removing code review requirements for some changes

[rtyler]

I shared this in Slack but am copying it here for others to participate in the thinking if they do not frequent Slack.

“what if we didn’t require code review for merging into main”

I’m exploring the thought more about what we might need to make that happen. “Why would you do such a thing, code review is so valuable!” I do find code reviews valuable but we do seem to lose a lot of flow time due to timezones, differing work schedules, and a number of other things. For something without a lot of changes, especially bug fixes that come with tests I would be much more comfortable with maintainers merging once CI goes green.

Some pieces of the puzzle that I think would be needed:

• Soft caps on pull requests. I saw this mentioned somewhere else, but implementing a soft cap of <500 lines per pull request can help people avoid massive unreviewable changes that are simpler to integrate.
• Incorporating some of the benchmarking work into CI that has already been explored. If performance of key operations is not affected and the build is green, go for it.
• Stronger semantic version checks: if our APIs have not changed and all tests pass, I’m generally comfortable with landing stuff by maintainers.
• Implementing Apache Software Foundation style release candidates and voting: this is where we would put a mandatory bottleneck, rather than some jokey slack emojis like I tend to do, implementing a true release candidate process that requires review and vote before we push something to users.

All of this is to say that reviews can still be requested, but I would love to see us land more improvements faster and I think we have a bunch of different schedules that can make pushing each change through a review queue a lot slower than necessary.

@ion-elgreco and @ethan-tyler expressed some supportive perspectives, in particular this from @ethan-tyler I am noodling 🤔

( read a blog post a while back sharing a framework of categorizing the PRs by the type of change. It was something like ship (merge once CI passes, no review needed), show (merge immediately but leave the PR open for async feedback), or ask (block on review before merge). We could look at deploying a bot can look at the diff, classify what kind of change it is and surface a risk level. I think this would allow us to be more specific on the type of change and not need a soft cap of <500 lines diff.

On benchmarks - the bot is at a point where we can deploy on dedicated infra and wire it in as a GitHub App or configure a webhook to trigger on demand. Don't think every PR needs a benchmark run and we can tease out the right strategy. I know datafusion manually triggers theirs from a white list of users who call the bot in the PR. Happy to hash that out.

[ethan-tyler]

For categorizing PRs- it doesn't need to be anything fancy. A github action that looks at which paths changed in the diff, checks the size, and applies a label (ship / show / ask) would get us most of the way. We don't need AI or external service, just path matching heuristics against our crate structure. Maintainers can override the label if the bot gets it wrong. Same action could tag PRs that touch core read/write paths as perf sensitive and trigger benchmark runs only for those plus manual triggers.