[Poll] What is your preferred way to Pull Request review?

Continuing the discussion from Would someone love to rewrite top answers on audio issues to Quick Docs articles?:

Hi all, I’d like feedback across different working groups/SIGs about your preferred way to PR review (examples);

  • Documentation comments and technical review
  • AsciiDoc attributes, markup, CSS, technical accuracy, stylistic improvements, code snippets

I take an example from Quick Docs in Pagure repositories and ask your opinions on how PR would be commented on. Note the question is asked from the point of view of the person receiving the PR and reviewing it.

Which one of the following review option would you use? The poll is open until 2024-02-29T18:00:00Z

  • Comment box (Image 1)
  • In-line comment field (Image 2)
  • It depends. I want to make a suggestion!
0 voters

Image 1: Comment box

Image 2. In-line comment field

Note: If you have better ideas, please share yours here. Thanks!

Hi Hank, I am a little confused. Option 1 is an in-line comment actually. You can tell from Ankur’s comment in the first screenshot because it shows the line number that his comment is attached to at the top of his comment box. Could you clarify the ask better?

If it is what I think it is, I prefer to leave comments in-line for specific feedback, but I also use the main comment box sometimes if I have more general feedback about the changes as a whole. Which type of comments I leave depends on the context of the change in question.

I don’t understand how the options are related - surely options 1 and 2 are from the point of view of the person receiving the PR and reviewing it while option 3 is the opposite, from the point of view of a submitter deciding whether to open a PR or skip that and push directly?

Change the image 1 and added a text around whose point of view. Sorry for causing confusion.

Absolutely, spot-on. I believe this is not widely followed on.

I removed submitter’s perspective. Thanks for correcting it.

If I wanted to submit a suggestion I’d definitely go for #2.

Both?

It depends if you are making general comments about the PR or if you are commenting on something specific.

I went ahead and voted for the in-line option since I do that most often, but I agree with @dalto that this better describes my approach. More about intention, less about tooling :slightly_smiling_face:

+1 to both.

When I’m reviewing the contents of the changes in the PR, I tend to use in-line comments. Note that this isn’t ideal on Pagure, though (unless this has changed since the last time I did it)—each time you submit an in-line comment, it’ll refresh the page and you end up back on the main PR page, and then you have to go to the “Files changed” tab again and find where you’d left off. GitHub/GitLab handle it better—you remain on the “Files changed” tab there so you can continue adding more comments.

When there’s general discussion about the PR, not the details of what has changed, I often drop a general comment.

+1 to both also.

In particular, because of the shortcomings in pagure that @ankursinha described, I find it hard to consume inline comments, because pagure gives a “line number”, and it’s not always easy to figure out where the comment was supposed to apply. (I think that this line number is also calculated incorrectly, and if somebody updates the PR, it will change anyway, so it’s not a good identifier.)

So I use an email-quoting like method, putting a “>” and some text that I want to comment on, and then the comment. That way it’s nicer for the person reading the comments.

1 Like

+1 both

When commenting on something specific in the submission, I use inline comments and the normal comment otherwise.

3 Likes