isaacs / github

Just a place to track issues and feature requests that I have for github
2.21k stars 129 forks source link

Pull request: no option to add to review when replying to thread in timeline #1543

Open OliverJAsh opened 5 years ago

OliverJAsh commented 5 years ago

In the pull request timeline, e.g. https://github.com/OliverJAsh/github-pr-test/pull/1, if I try to reply to a thread, I have no option to add my reply comment to a review. Rather the only option is to send the comment immediately (not part of a review):

image

If I click through to the diff and then try to reply to the same thread there, I do have the option to add my reply comment to a review:

image

I would expect to be able to add thread replies to a review in the pull request timeline, just like I can when viewing the diff. This is because: it can be very difficult to find a thread in a diff, especially if that diff is large, or the thread is outdated and can only be found in an old diff.

At the very least I would expect a link in the pull request timeline so I can easily go to the thread in the diff and then reply from there.

TPS commented 5 years ago

I see a reply option there correctly, as this requests. Are you using mobile view?

OliverJAsh commented 5 years ago

reply option there correctly

What I expect: "start a review" (2nd screenshot) What I see: "comment" (1st screenshot)

Could you elaborate on what you see?

TPS commented 5 years ago

I think I see it: Iff 1 opens the review, 1 cannot immediately reply to 1's own review/comment in the indented subconversation, but is able to further comment in the same thread.

I don't really see how that's a problem, though. 🤷🏾‍♂️

OliverJAsh commented 5 years ago

I think you may be misunderstanding. You can always reply to a thread. The difference is:

I would expect consistent behaviour between the pull request timeline and diff.

twidi commented 5 years ago

Maybe it's because the diff is "outdated" in the first screen and you can't start a review on outdated diff? In the diff (files) view, you cannot see outdated diff, so it makes sense. So you should try with the same context

OliverJAsh commented 5 years ago

Maybe it's because the diff is "outdated" in the first screen?

Here's a test where the comment is not outdated. Same problem.

https://github.com/OliverJAsh/github-pr-test/pull/4#pullrequestreview-228913694

twidi commented 5 years ago

Here's a test where the comment is not outdated. Same problem.

ok fine! I had to ask to eliminate this case ;)