[llvm-dev] [cfe-dev] Phabricator -> GitHub PRs?
Daniel Sanders via llvm-dev
llvm-dev at lists.llvm.org
Thu Jan 23 12:42:29 PST 2020
> On Jan 23, 2020, at 08:37, David Greene via llvm-dev <llvm-dev at lists.llvm.org> wrote:
>
> Hubert Tong <hubert.reinterpretcast at gmail.com> writes:
>
>>> I read this as the refresh being an entirely new GitHub PR. Is that
>>> right? Normally I would expect the same PR to be used but the rebase
>>> would cause a force-push of the branch which would update the PR with
>>> the new commits but might lose comments. It's that later part I'm
>>> unsure about. It would seem odd to me to open an entirely new PR due to
>>> a rebase/update of commits to respond to review.
>>>
>> Use of force push damages the ability to retrieve context on older
>> comments. I am not sure of the reason for the case I observed, but the
>> context vanished within a week in one instance.
>
> Does "vanish" mean it was completely gone, or just hidden in some way?
I can provide a concrete example as I ran into this recently at https://github.com/pygments/pygments/pull/1361. If you scroll down that PR you'll see an entry 'pygments/lexers/asm.py Show resolved' (actually there's two, it doesn't matter which you pick). If you expand that by clicking 'Show Resolved' you'll see a small amount of context along with the reviewers comment. However, if you click the filename to look at the whole file you end up at https://github.com/pygments/pygments/pull/1361/files/626154e9498271420b5fddf54910f332d90626a6 which shows 'We went looking everywhere, but couldn’t find those commits.'. GitHub has already removed the previous version of the patch that the review comments referred to. The commit disappeared almost immediately after my force push.
> -David
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
More information about the llvm-dev
mailing list