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

[James Y Knight via cfe-dev]

Actually -- you have shown a slightly different issue -- this one is a UI
issue. The commit didn't actually get purged from the repository.......yet.
You can tell that by going to the commit directly,
https://github.com/pygments/pygments/commit/626154e9498271420b5fddf54910f332d90626a6
.

So the workaround I proposed wouldn't help this at all.

On Thu, Jan 23, 2020 at 4:07 PM James Y Knight <jyknight at google.com> wrote:

> 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/cfe-dev/attachments/20200123/82775c46/attachment-0001.html>