Hacker News new | ask | show | jobs
by kevincox 2053 days ago
I don't need perfect, I just need good. GitHub and GitLab both have good implementations as well as every other good code review system I have used. GitHub annoying tries its hardest to hide the "outdated" comments but GitLab has the option to keep them open (they are no longer visible in the code, but remain on the discussion tab)

So I appreciate your opinion that it is impossible, but as a reviewer I much prefer when the tool tries.

1 comments

GitHub's implementation does not do what you claim it does. GitHub has no behavior around porting and placing comments (while Phabricator does), GitHub just hides anything it can't place exactly. See my link above for a detailed description of GitHub's very simple implementation. I believe this is absolutely the wrong tradeoff.
I use GitHub every day, I've definitely seen it preserve some comments. Sure, it drops a lot. But I still prefer this to dropping them all, or showing the comments on the wrong lines.
I mean that GitHub does not "try", in the sense of looking at the interdiff, doing fuzzy matching, trying to identify line-by-line similarity, etc. It places comments only if the hunk is exactly unchanged and gives up otherwise.

Phabricator does "try", in the sense that it examines the interdiff and attempts (of course, imperfectly, because no implementation can be perfect) to track line movement across hunk mutations.

My claim is that all comments which GitHub places correctly, Phabricator also places correctly. And some comments which GitHub drops, Phabricator places correctly (on the same line a human would select)! However, some comments which GitHub drops, Phabricator places incorrectly (on a line other than the line a human would select).

So the actual implementation you prefer is not one that tries, but one that doesn't try! Phabricator could have approximately GitHub's behavior by just deleting a bunch of code.

That's perfectly fine: many other users also prefer comments be discarded rather than tracked to a possibly-wrong line, too. I strongly believe this isn't a good behavior for code review software, which is why Phabricator doesn't do it -- but Phabricator puts substantially more effort into trying to track and place comments correctly than GitHub does.