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
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?
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
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.
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.