“Good First Review” label #19941

issue fjahr openend this issue on September 11, 2020

fjahr commented at 2:14 pm on September 11, 2020: member
I would like to suggest a small idea that I had, to make it easier for beginner to review their first PRs. Analogous to the “Good first issue” label, I am suggesting a “Good first review” label.

As a general guideline, labeled pull requests should have some (ideally all) of the following characteristics:
- Complexity level low
- A tentative small change (<100 LoC)
- A detailed description of the PR background if needed
- General instructions/hints on how to test/reproduce
Examples of pulls that generally tend to be good first reviews:
- A newly added RPC that can be easily manually tested to verify functionality
- A PR that only adds new functional test coverage since these tend to be self-documenting
- A documentation change that can be verified to be correct with manual testing
Similar to other labels the creator of the pull could ask for the label to be applied or contributors with the power to give labels could apply the proactively.
fjahr added the label Feature on Sep 11, 2020
pinheadmz commented at 2:25 pm on September 11, 2020: member

Great idea – wanna pull some suggestions from the current open PRs as examples?
fanquake added the label Brainstorming on Sep 11, 2020
michaelfolkson commented at 3:14 pm on September 11, 2020: contributor
Concept ACK. Those characteristics are good too.

I would suggest some additions.
- PR opened by a (semi-) experienced reviewer (e.g. @fjahr, @pinheadmz) who can provide guidance on how to review the PR in the initial comment and answer questions. I don’t know if a good first issue PR/new contributor’s PR is a good candidate for this.
- PRs that people won’t mind being noisier in the comments than most.
- PRs that can be left open for a sufficient time to give new reviewers time on them before being merged. Any PRs that are urgent or holding up other PRs being merged are obviously not good candidates for this.
jonatack commented at 4:22 pm on September 11, 2020: member
Concept ACK on encouraging review by new contributors.

Perhaps:
- “A newly added or updated RPC or CLI option that can be easily tested to verify functionality and help documentation”
- “A PR in which the author provides clear information in the description to reviewers on how to test and review”
If a template is created, perhaps add links to CONTRIBUTING.md and possibly also doc/developer_notes.md and doc/productivity.md.
fjahr commented at 11:04 am on September 13, 2020: member
If a template is created, perhaps add links to CONTRIBUTING.md and possibly also doc/developer_notes.md and doc/productivity.md.

TBH, I didn’t think of making it a template. We only have one pull template so far and a new one just for this label might be unnecessary. I will propose doc changes if the label gets created.

I don’t know if a good first issue PR/new contributor’s PR is a good candidate for this.

Makes sense. My thought was that looking for a well-done description and guidance on how to test in the description would probably mean the pull is from someone with a little more experience. Generally, the label probably shouldn’t be applied unless the author agrees or asks for it.

wanna pull some suggestions from the current open PRs as examples?

I think these might be good candidates:
- #19877
- #19883
- #19253
jonatack commented at 11:32 am on September 13, 2020: member

TBH, I didn’t think of making it a template. We only have one pull template so far and a new one just for this label might be unnecessary. I will propose doc changes if the label gets created.

Agree.
michaelfolkson commented at 12:54 pm on September 13, 2020: contributor

I wouldn’t overcomplicate this. It is a very low risk experiment and we don’t really know what will work until we try it. I agree that those PRs look like good candidates. Should probably stick to absolute maximum of 3 to begin with.
pinheadmz commented at 1:12 pm on September 17, 2020: member
Concept ACK – are issue/PR labels something triage-level contributors can do? Does anyone on this thread have that permission? (out of curiosity)
I think these might be good candidates:

#19877

#19883

#19253
Agree with all three of these - small changes in small number of files. Excellent comments by author, even just visually the diffs are simple and easy to look at. More python than C++ :-) and even in #19883 which is more C++, it’s very simple.
MarcoFalke added the label Good First Review on Oct 2, 2020
MarcoFalke commented at 9:40 am on October 2, 2020: member

I’ve created the label (used the color from the “Review club” label)
fjahr closed this on Oct 2, 2020
fjahr commented at 9:17 pm on October 2, 2020: member

Closed since we have the label now and I will make a brief announcement on IRC
DrahtBot locked this on Feb 15, 2022

Contributors
fjahr pinheadmz michaelfolkson jonatack MarcoFalke

Labels
Feature Brainstorming Good First Review