[llvm-dev] [cfe-dev] Phabricator -> GitHub PRs?

Hubert Tong via llvm-dev llvm-dev at lists.llvm.org
Thu Jan 23 15:57:55 PST 2020


On Thu, Jan 23, 2020 at 6:44 PM Doerfert, Johannes <jdoerfert at anl.gov>
wrote:

> On 01/23, Hubert Tong wrote:
> > On Thu, Jan 23, 2020 at 11:37 AM David Greene <greened at obbligato.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?
> > >
> > Completely gone.
>
> That seems an unfortunate design choice. It arguably can make the code
> review hard and awkward, but even if only "done" comments vanish the
> discussion that lead to the design might be lost.
>
> Maybe they (=GH) can add a way to recover comments (without too much
> effort)?
>
There's two layers to this. The complete loss of commits (for which James
indicated a solution for) and the difficultly of working with comments made
on commits that are no longer connected to the PR branch. Improvements to
the overall user experience on GitHub PRs for rebase-based PR refreshing
seems to be the ask.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200123/6908fa96/attachment.html>


More information about the llvm-dev mailing list