I don’t think we can presume everyone has the same working style. For myself
I’ll happily review a PR that has a failing check. I’m OK if it has some
innocuous ‘housekeeping’ error or unrelated failure.
I don’t retrigger PR failures, for unrelated errors, just to ‘get to green’ –
related, I don’t expect anyone to do that on my part either. It would be
frustrating if I was about to merge something and someone retriggers a job. Yes
I do merge if I’m 100% confident the failed check is unrelated. I don’t merge
if any checks are still pending.
Perhaps this is just relevant to my current situation, but most of my PRs are
module specific and so there is collaboration between my team and we typically
know the state of our various PRs. I don’t feel like there is much need for any
process around switching in and out of Draft mode. Much less for an ‘external’
contributor to make decisions on our behalf.
Has some situation arisen that is driving this? It feels like there is some
underlying issue that isn’t being fully communicated.
--Jens
From: Owen Nichols <onich...@vmware.com>
Date: Thursday, May 6, 2021 at 9:12 AM
To: dev@geode.apache.org <dev@geode.apache.org>
Subject: Re: Reminder to use draft mode
A PR in "Draft" mode simply conveys that at least one more commit is coming
before it will be "done". Reviewers generously volunteer their time to look at
your PR, and are welcome to look at it while in draft mode if they wish, but if
they are quite busy, some may prefer to wait until the PR is plausibly
code-complete before setting aside time to review it.
Sorry if I wasn't clear, I don't mean that flaky failures should mean a PR is
not done. You can always refer to the latest mass test report for a list of
known flaky failures, but often I will see those and retrigger them for you
anyway.
I expect that most PR submitters will be monitoring their own PR checks and
taking it back to draft mode as soon as they realize more changes are needed.
But if as a community we agree to use draft mode to communicate status in this
way, it shouldn't matter who does it.
Due to CODEOWNERS, some reviewers have a huge number of PRs in their queue.
Clearly communicating the status of your PR allows reviewers to focus their
time on PRs that are ready for review.
On 5/6/21, 8:51 AM, "Jens Deppe" <jde...@vmware.com> wrote:
Comments inline…
Please keep your PR in draft mode anytime it is not ready to be reviewed.
This includes if you have received request for changes, or if any PR checks
are not passing.
How do I know if everyone is done reviewing? Or even who might be
reviewing? Different reviewers may be looking at different areas, depending on
the scope of the change. If the PR suddenly switches back to ‘Draft` what does
that mean if I’m reviewing it? Worse still, if I’m the owner and someone else
switches it to Draft I’m not notified.
Additionally, many PR checks fail for reasons unrelated to the PR so
switching blindly to ‘Draft’ seems pointless.
If you’re reviewing someone’s PR, and notice any checks not passing or you
are requesting changes, please also click “Convert to draft”.
I really don’t agree with this – if you have an issue with a PR for
whatever reason, please respect the author and address it directly with them. I
certainly feel uncomfortable ‘messing’ with someone else’s PR and, by the same
token, don’t want my PRs adjusted without my input.
--Jens
