<div dir="ltr"><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jan 23, 2020 at 6:44 PM Doerfert, Johannes <<a href="mailto:jdoerfert@anl.gov">jdoerfert@anl.gov</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">On 01/23, Hubert Tong wrote:<br>
> On Thu, Jan 23, 2020 at 11:37 AM David Greene <<a href="mailto:greened@obbligato.org" target="_blank">greened@obbligato.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>
> Completely gone.<br>
<br>
That seems an unfortunate design choice. It arguably can make the code<br>
review hard and awkward, but even if only "done" comments vanish the<br>
discussion that lead to the design might be lost.<br>
<br>
Maybe they (=GH) can add a way to recover comments (without too much<br>
effort)?<br></blockquote><div>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.<br></div></div></div>