Skip to content
This repository was archived by the owner on Aug 22, 2019. It is now read-only.

Last actor information not always correct #1

Open
whimboo opened this issue Mar 6, 2014 · 2 comments
Open

Last actor information not always correct #1

whimboo opened this issue Mar 6, 2014 · 2 comments

Comments

@whimboo
Copy link

whimboo commented Mar 6, 2014

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.

@peterbe
Copy link
Owner

peterbe commented Mar 6, 2014

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.

@whimboo
Copy link
Author

whimboo commented Mar 6, 2014

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants