Conventions and practices regarding Code Review on PRs, and subsequent approval #8431
Replies: 2 comments
|
This is an important topic, and there were several previous discussions (both online and in person) about it. As a new community, we need to gradually identify a good practice for us to follow. Here I share my own thoughts:
|
|
AI can help make the review loop faster, but the author still owns the quality of the PR and the reviewer still needs to understand the design. One thing we’ve found useful is adding a production risk signal to that process. Tomosu’s PRI looks at the change, dependencies, and production context to give the PR a risk score, so reviewers have another signal when deciding where to dig deeper. Could be a useful complement to AI review tools. |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Hi folks,
Yesterday there was a casual discussion between a large group of contributors, PMC members and Committers
about Code Review, how it is currently practiced in Texera, and how that has been going so far. I would
like to serialize that discussion and bring it to the broader community because I think there were some
important insights and questions that arose. Putting on my Mentor hat for a moment, it is vital that
these kinds of development-related decisions are eventually held in an open forum.
First, let me preface some of this with some ASF-related stuff (that you can skip if you are familiar).
This is simply my understanding, if someone knows better please correct me.
There is no specified or required practice for Code Review within an ASF project. It is for each community
to decide. As far as the ASF goes, There are simply 3 roles: Contributor, Committer, and PMC member.
The latter 2 in principle have demonstrated the care and responsibility to the codebase to be entrusted to
uphold the community principles with regards to committing new code to the project, whatever they are.
What should the role of CR be?
Should code review be a thorough and comprehensive check by a subject expert, or should it simply
be a brief check by a colleague? Or somewhere between the two? Should it depend on the area or
some other factor?
Under what circumstances can a PR be approved?
Sometimes there is an ideal reviewer for a PR, but they are not available. In these circumstances,
can someone else step in? If they do, is this something that can always be done, or only in certain
situations?
How should the work of CR be distributed?
It naturally is the case that the most experienced people in the project are often the best one to
review a PR. However concentrating all of this work on a few members leads to a bus factor, which
can frustrate everyone by delaying merges and overburdening senior members. So ideally there is
at least some principle by which to parallelize, or at least time share, this task among a limited
set of resources.
I think there are already some implicit and informal conventions. It would be good to attempt to
clarify and make these clear, so that new contributors can understand immediately rather than
having to observe and infer
I will refrain from giving my view, I am most looking forward to hearing from everyone else first.
Of course if there is a general question regarding this that someone feels I am forgetting, please
also feel free to raise it.
All reactions