github-pr-triage icon indicating copy to clipboard operation
github-pr-triage copied to clipboard

Last actor information not always correct

Open whimboo opened this issue 11 years ago • 2 comments

When I checked this tool today (which is indeed great!) I have seen that at least for the following case the last actor information is not correct:

https://prs.paas.allizom.org/mozilla/mozmill-ci

Pull Update mozmill-ci to run metro testruns

This references Andreea via this comment whereby it is 13 days old and I responded yesterday.

whimboo avatar Mar 06 '14 07:03 whimboo

It's actually correct. Its design is just questionable. If you expand it you'll see that the last comment was by Andreea. screenshot 2014-03-06 08 23 55

What's causing the confusion is the differentiation between pull comments and issue comments. Issue comments are comments on pull requests that aren't attached to a particular line in a diff.

The reasoning is that a "pull comments" are less definitive. Meaning you might post 2 pull comments and 1 issue comment. E.g.

  1. (pull comment) This line needs some work
  2. (pull comment) This is a good idea!
  3. (issue comment) In general it looks good. Some nits. r+ if you address the nits.

Basically, if you sit down to review a PR, hopefully you leave a final general comment when you've concluded your work and intend to walk away from it for the moment.

I was worried that that would be misunderstood. I think we need to sleep on it a bit.

peterbe avatar Mar 06 '14 16:03 peterbe

The so called pull comments are shown in sequence with the general comments when you open a PR page. So I never made such a distinction yet, or better recognized that those should be differently handled.

whimboo avatar Mar 06 '14 17:03 whimboo