<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
{font-family:"Cambria Math";
panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.msonormal0, li.msonormal0, div.msonormal0
{mso-style-name:msonormal;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
span.EmailStyle18
{mso-style-type:personal-reply;
font-family:"Calibri",sans-serif;
color:windowtext;}
.MsoChpDefault
{mso-style-type:export-only;
font-family:"Calibri",sans-serif;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><b>From:</b> cfe-dev <cfe-dev-bounces@lists.llvm.org> <b>On Behalf Of
</b>Hubert Tong via cfe-dev<br>
<b>Sent:</b> Thursday, June 25, 2020 7:51 AM<br>
<b>To:</b> Zachary Turner <zturner@roblox.com><br>
<b>Cc:</b> LLVM Dev <llvm-dev@lists.llvm.org>; Nikita Popov <nikita.ppv@gmail.com>; Clang Dev <cfe-dev@lists.llvm.org><br>
<b>Subject:</b> Re: [cfe-dev] [llvm-dev] Phabricator Maintenance<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<div>
<p class="MsoNormal">On Thu, Jun 25, 2020 at 6:04 AM Zachary Turner via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal">On Thu, Jun 25, 2020 at 2:43 AM Nikita Popov <<a href="mailto:nikita.ppv@gmail.com" target="_blank">nikita.ppv@gmail.com</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<div>
<p class="MsoNormal">On Thu, Jun 25, 2020 at 11:22 AM Zachary Turner via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<div>
<div>
<p class="MsoNormal">I can’t really provide a doc, but i can describe what I believe to be the biggest problem.<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">In a GH PR, comments are associated with commit hashes. If a commit hash ceases to exist, so do all comments associated with it. The comments are quite literally destroyed and irretrievable.<o:p></o:p></p>
</div>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Either I'm misunderstand something, or this is just blatantly incorrect. Assuming we're talking about pull request reviews here, review comments do not get lost, regardless of how you rebase the pull request branch.<o:p></o:p></p>
</div>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal">Try this experiment:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">1. Create a PR<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">2. Have someone leave a comment on a line of code. You should get an email.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">3. Make another change locally and squash it<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">4. git push -f<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">5. Go to your email and click the link for the comment that was made in step 2.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">GH should tell you that the comment does not exist.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Now that i think about it, the comment is probably still visible in the web ui by clicking “Show Outdated”. But you cannot see the code that was requested to be changed. That comment has a link associated with it, and if you click that
link you just get “the commit does not exist”<o:p></o:p></p>
</div>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal">In other words, the context is gone. The state that the reviewer saw is no longer necessarily retrievable (via GitHub).<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Perhaps that is mitigated by the e-mail archive, which could make it possible to reconstruct that context.<o:p></o:p></p>
<p class="MsoNormal"><b><i>[Keane, Erich] Well, you get a preview of the line commented on/surrounding lines, but yes, the history disappears. That said, we explicitly discourage force-push on my downstream for exactly this reason. I don’t know if there is
a way to, but it would be wonderful if we could just prevent force-pushes from happening on reviews.</i></b><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<blockquote style="border:none;border-left:solid #CCCCCC 1.0pt;padding:0in 0in 0in 6.0pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal">_______________________________________________<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" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev</a><o:p></o:p></p>
</blockquote>
</div>
</div>
</div>
</body>
</html>