There are some pieces of Apache infrastructure we can control without needing
tickets: Specifically
required_pull_request_reviews:
dismiss_stale_reviews: true
require_code_owner_reviews: true
I think these specific controls could go a long way towards helping keep our PR
quality high, while reducing cognitive load for the reviewers.
Full documentation
here<https://cwiki.apache.org/confluence/display/INFRA/git+-+.asf.yaml+features#git.asf.yamlfeatures-BranchProtection>
From: Bruce Schuchardt <[email protected]>
Date: Wednesday, October 28, 2020 at 8:10 AM
To: [email protected] <[email protected]>
Subject: Re: PR process and etiquette
-1
While I often use the Draft option I don't see why we want to add even more
rules about how we use github. I think it's enough to put in a PR and then add
reviewers when you're ready for comments. Getting the stink-eye for putting up
a non-Draft PR is just going to make it more difficult to attract and retain
new contributors.
On 10/27/20, 5:41 PM, "Udo Kohlmeyer" <[email protected]> wrote:
Dear Apache Geode Devs,
It is really great going through all the PRs that been submitted. As Josh
Long is known to say: "I work for PRs".
Whilst going through some of the PRs I do see that there are many PRs that
have multiple commits against the PR.
I know that the PR submission framework kicks off more testing than we do
on our own local machines. It is extremely uncommon to submit a PR the first
time and have all tests go green. Which h means we invariably iterate over
commits to make the build go green.
In this limbo time period, it is hard for any reviewer to know when the
ticket is ready to be reviewed.
I want to propose that when submitting a PR, it is initially submitted as a
DRAFT PR. This way, all test can still be run and work can be done to make sure
"green" is achieved. Once "green" status has been achieved, the draft can then
be upgraded to a final PR by pressing the "Ready For Review" button. At this
point all commits on the branch can then once again be squashed into a single
commit.
Now project committers will now know that the PR is in a state that it can
be reviewed for completeness and functionality.
In addition, it will help tremendously helpful if anyone submitting a PR
monitors their PR for activity. If there is no activity for a few days, please
feel free to ping the Apache Geode Dev community for review. If review is
request, please prioritize some time to address the feedback, as reviewers
spend time reviewing code and getting an understanding what the code is doing.
If too much time goes by, between review and addressing the review comments,
not only does the reviewer lose context, possibly requiring them to spend time
again to understand what the code was/is supposed to do, but also possibly lose
interest, as the ticket has now become cold or dropped down the list of PRs.
There are currently many PRs that are in a cold state, where the time
between review and response has been so long, that both parties (reviewer and
submitter) have forgotten about the PR.
In the case that the reviews will take more time to address than expected,
please downgrade the PR to draft status again. At this point, it does not mean
that reviewers will not be able to help anymore, as you can request a reviewer
to help with feedback and comments, until one feels that the PR is back in a
state of final submission.
So, what I'm really asking from the Dev Community:
If you submit a PR, it would be great if you can nudge the
community if there is no review on the PR. If feedback is provided on a PR,
please address it as soon as possible. This not only will help the PR progress
faster, but it will shorten the feedback loop.
Finally, start with a DRAFT PR and then upgrade to a final PR when
you feel it is in a "good" state. If more work is required, it is ok to
downgrade back to a draft, until one feels the PR is in a good state again.
Managing the PR process in this manner, will be hugely beneficial for all
community members. As reviewers will know when a PR is ready to be reviewed.
Reviewers will also feel more engaged in this process, due to timely response
to their review comments. PR submitters will feel happier, in the knowledge
that the code they spent so long meticulously crafting will possibly make it
into the project.
Any thoughts?
--Udo
