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

[Daniel Sanders via cfe-dev]

Ah ok. So it was only removed from their representation of a pull request. It's not ideal as you can see either the comments or the file but not both at the same time but at least both are still available.

> On Jan 23, 2020, at 13:10, James Y Knight <jyknight at google.com> wrote:
> 
> 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 <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 <mailto: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 <mailto: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 <mailto:llvm-dev at lists.llvm.org>> wrote:
> > 
> > Hubert Tong <hubert.reinterpretcast at gmail.com <mailto: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 <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 <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 <mailto:llvm-dev at lists.llvm.org>
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
> 
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev <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/20200124/c1457d9e/attachment.html>