<div dir="ltr"><div>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. </div><div><br></div><div>(I have the uneasy feeling it will not be, due to security restrictions)<div><div><br></div><div>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.</div></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jan 23, 2020 at 3:43 PM Daniel Sanders via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
<br>
> On Jan 23, 2020, at 08:37, David Greene via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br>
> <br>
> Hubert Tong <<a href="mailto:hubert.reinterpretcast@gmail.com" target="_blank">hubert.reinterpretcast@gmail.com</a>> writes:<br>
> <br>
>>> I read this as the refresh being an entirely new GitHub PR. Is that<br>
>>> right? Normally I would expect the same PR to be used but the rebase<br>
>>> would cause a force-push of the branch which would update the PR with<br>
>>> the new commits but might lose comments. It's that later part I'm<br>
>>> unsure about. It would seem odd to me to open an entirely new PR due to<br>
>>> a rebase/update of commits to respond to review.<br>
>>> <br>
>> Use of force push damages the ability to retrieve context on older<br>
>> comments. I am not sure of the reason for the case I observed, but the<br>
>> context vanished within a week in one instance.<br>
> <br>
> Does "vanish" mean it was completely gone, or just hidden in some way?<br>
<br>
I can provide a concrete example as I ran into this recently at <a href="https://github.com/pygments/pygments/pull/1361" rel="noreferrer" target="_blank">https://github.com/pygments/pygments/pull/1361</a>. 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 <a href="https://github.com/pygments/pygments/pull/1361/files/626154e9498271420b5fddf54910f332d90626a6" rel="noreferrer" target="_blank">https://github.com/pygments/pygments/pull/1361/files/626154e9498271420b5fddf54910f332d90626a6</a> 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.<br>
<br>
> -David<br>
> _______________________________________________<br>
> LLVM Developers mailing list<br>
> <a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
> <a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
<br>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><br>
</blockquote></div>