Dear GitHub, you are missing such a basic concept in ways that are incredibly frustrating #17481
Replies: 7 comments 2 replies
-
I'm glad to see that there is an unclosed version of this closed community discussion from 2019 out here. Linking this conversation to that one will hopefully give GitHub a better feel for just how long and badly this "Intended Functionality" has been annoying their actual users. |
Beta Was this translation helpful? Give feedback.
-
The "differential" tool in Phabricator (and its successor Phorge, I assume) has great support for this. An "outdated" comment thread generally stays with the line of the file it was associated with across edits and file renames, on a best-effort basis. Sometimes the line is no longer there, and the comment will show up near where it was made, but confusing without the right context. Still, it's useful that it is easy to find, and it is easy to collapse if no longer helpful. In every case, an "outdated" comment is shown in a grey color, so you know it's outdated (applies to a different version of this code). And in every case, it includes a ⏪ icon to open the diff to the commit that the comment was made on, in which case you'd see it in its original non-outdated context. It also has useful logic for which side of the side-by-side diff to show the comment on, basically choosing the closer commit to the one that the comment was made on. This means in particular that non-outdated comments are always shown on the expected side in the correct context. A related super-useful feature is keyboard shortcuts to go to next( This is all to say that the requested behavior is entirely possible, and I too am missing it greatly in Github. |
Beta Was this translation helpful? Give feedback.
-
Bump, this is a really big UX issue, enough to consider moving to Gitlab or Bitbucket which do manage this |
Beta Was this translation helpful? Give feedback.
-
I agree that GitHub should change this and make outdated comments an optional feature. |
Beta Was this translation helpful? Give feedback.
-
This issue is now documented in at least four places with no real official response.
|
Beta Was this translation helpful? Give feedback.
-
As a longtime user of Phabricator (Differential) internally at FB, I desperately miss this UX. The flow described in the original post on this issue is both basic and crucial, and without proper support for it, it takes two windows to review requested changes on GitHub (and even then, it's a lot more work). Please fix this GitHub! Without it, I'm likely to end up switching back to Phabricator (now Phorge). |
Beta Was this translation helpful? Give feedback.
-
I guess we need to bump all a bit more, it's the most annoying thing coming over gitlab, initial context of seeing how the new change 'fixes it' it's important. |
Beta Was this translation helpful? Give feedback.
-
Its a simple concept, and it shouldn't matter if the PR is large or small, but it does get exponentially worse with larger PR's
As of the time of this post, Bob cannot make this determination easily.
The conversations tab where the comments all appear just shows "Outdated" and the commit that he added that comment. It does not show the updates that Joe did that made the comments outdated.. It looks like GH expects Bob to click the Resolve Conversation button on this conversation without seeing the code that supposedly resolved it.
The files changed tab is equally vexing. Bob can select the latest commit, but that does not show any of the three conversations on it. If he selects a conversation, the commit he selected is automatically changed to the one that originated the comment and he no longer sees the updates that Joe made. There is no way to see the comment and the updated code together unless Bob opens two browser windows to view the PR; one for the comment, and one to see the changes.
The only thing I can rationalize is that you expect us to treat prior conversations as if they do not exist and treat each review as if it were brand new like a TFVC code review, except that the tools here dont give that impression.
Beta Was this translation helpful? Give feedback.
All reactions