[llvm-dev] [cfe-dev] Phabricator -> GitHub PRs?
David Greene via llvm-dev
llvm-dev at lists.llvm.org
Wed Jan 22 14:19:12 PST 2020
Hubert Tong <hubert.reinterpretcast at gmail.com> writes:
> It was already mentioned earlier in the thread that:
> - The individual commits cannot be approved in the sense of approving a PR.
Later conversation makes me wonder how often that really happens though.
In a true patch series I would think it would be very rare to be able to
land a later commit before an earlier commit.
> - Comments on individual commits are easily lost.
This is the piece that I feel needs more explanation. What does "lost"
mean, exactly? I will comment more on your explanation below.
> To elaborate on the latter:
> - GitHub would not show comments in-line if it unilaterally believes that
> the comment no longer applies to the version of the code you are looking at.
Ok, but doesn't Phab do the same? Or does Phab attach the comment to
some potentially completely inappropriate line? The latter seems worse
> - GitHub would proactively hide and collapse comments in the "conversation
> view", especially if it believes (for such bad reasons as line noise in
> later commits) that an earlier comment is not relevant.
I'm reading this as comments aren't "lost" in the sense that they are
completely deleted from the PR. They may be collapsed. What does
"hide" mean? Are they completely inaccessible?
This is an important point. If comments are simply collapsed and I can
easily get at them if I need to, that's not too much of a problem for
me. After all, Phab auto-hides comments all the time if the
conversation gets "too long." I am frequently annoyed by having to
un-collapse them. Sounds like I would be annoyed in the same way with
GitHub, so nothing really gained or lost to me.
More information about the llvm-dev