How about we make this a recommendation rather than a rule? I'd like to also recommend that contributors consider prefixing the PR title with "DRAFT: " while it is in draft. This just makes it easier to see at a glance that it's a draft. When I change the PR to "ready for review" I edit the title to remove "DRAFT: ".
On Wed, Oct 28, 2020 at 9:59 AM Bruce Schuchardt <bru...@vmware.com> wrote: > Hi Owen - I wasn't aware that non-committers can't add reviewers to their > PRs but I don't see how using DRAFT mode helps with that. The idea that I > can't request a review until the commit checks all pass seems absurd to me. > > On 10/28/20, 9:15 AM, "Owen Nichols" <onich...@vmware.com> wrote: > > Hey Bruce, please consider that non-committers are not permitted to > add reviewers themselves, so a consistent convention to indicate when a PR > has moved from work-in-progress to ready-for-review will help alert the > community when to assign reviewers. > > Currently, I see countless creative variations on people adding "WIP" > or "DO NOT REVIEW" or other text somewhere in the summary or description. > I think the suggestion here is to standardize on using Draft status as the > canonical way to communicate this. > > Also worth noting, you can change a PR back to 'draft' mode at any > time (so whether you forgot initially, or a test failure or review feedback > made you realize more work is needed, you can always amend the draft status > to communicate whether you're still working, or ready for review again). > > I'm not sure why you think this is going to make it more difficult to > attract and retain new contributors. The point is to make it easier for > new contributors to get their PRs reviewed, and give reviewers more > confident that their time is appreciated. > > I see your point that as a committer, you have no need to communicate > this status, since you can simply add reviewers yourself when you're > ready. Asking everyone to use draft status is not the ideal workaround for > the GitHub flaw that non-committers can't request reviewers, but it also > takes only 2 seconds of your time. Expecting new contributors to email the > dev list every time they want to request a review of their PR seems far > more problematic in term of attracting and retaining new contributors. > > On 10/28/20, 8:10 AM, "Bruce Schuchardt" <bru...@vmware.com> wrote: > > -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" <u...@vmware.com> 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 > > > > >
