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

[James Y Knight via llvm-dev]

I started to try to write a workaround for this problem using a github
action a couple months ago. The idea is to backup up any ref which is
force-pushed, so the commits cannot get garbage collected. I tested it a
tiny bit, but I didn't get around to testing is whether the action will be
appropriately triggered, with an appropriately-powered secret token, in the
case of a pull request update from a user who is not a committer in the
repository.

(I have the uneasy feeling it will not be, due to security restrictions)

But, we could certainly deploy something like this, as an external hook if
a github action doesn't work. IMO that would be a requirement if we switch
to Github Pull Requests, so as not to lose any of the old pull-request
commit revisions. Ideally, github would just fix this themselves -- but it
is within our power to do so if they do not.

On Thu, Jan 23, 2020 at 3:43 PM Daniel Sanders via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

>
>
> > 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
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200123/7a7dce37/attachment.html>