<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:SimSun;
panose-1:2 1 6 0 3 1 1 1 1 1;}
@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;}
@font-face
{font-family:"\@SimSun";
panose-1:2 1 6 0 3 1 1 1 1 1;}
@font-face
{font-family:"Bookman Old Style";
panose-1:2 5 6 4 5 5 5 2 2 4;}
@font-face
{font-family:FangSong;
panose-1:2 1 6 9 6 1 1 1 1 1;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman",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;}
span.EmailStyle17
{mso-style-type:personal-reply;
font-family:"Bookman Old Style",serif;
color:#1F497D;
text-decoration:none none;}
.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"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif;color:#1F497D">Hi Dean,<o:p></o:p></span></p>
<p class="MsoNormal" style="text-indent:12.0pt"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif;color:#1F497D">Please check my below interleaved reply.<o:p></o:p></span></p>
<p class="MsoNormal" style="text-indent:12.0pt"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif;color:#1F497D">Thanks for your suggestion.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif;color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif;color:#1F497D">B.R.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif;color:#1F497D">Changcheng<o:p></o:p></span></p>
<p class="MsoNormal"><a name="_MailEndCompose"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif;color:#1F497D"><o:p> </o:p></span></a></p>
<p class="MsoNormal"><a name="_____replyseparator"></a><b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif">From:</span></b><span style="font-size:11.0pt;font-family:"Calibri",sans-serif"> Dean Troyer [mailto:dtroyer@gmail.com]
<br>
<b>Sent:</b> Friday, December 14, 2018 12:36 AM<br>
<b>To:</b> Liu, Changcheng <changcheng.liu@intel.com><br>
<b>Cc:</b> Xie, Cindy <cindy.xie@intel.com>; starlingx-discuss@lists.starlingx.io<br>
<b>Subject:</b> Re: [Starlingx-discuss] Notes: Weekly StarlingX non-OpenStack Distro meeting, 12/12<o:p></o:p></span></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<div>
<p class="MsoNormal">On Wed, Dec 12, 2018 at 8:06 PM Liu, Changcheng <<a href="mailto:changcheng.liu@intel.com">changcheng.liu@intel.com</a>> wrote:<span style="font-size:10.0pt;font-family:"Bookman Old Style",serif"> </span><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"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif"> 2.
<b>[Dean] the references to the original commit are not available externally</b><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif"> [Changcheng] Yes. The reference should be removed from commit message at last.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif"> Originally, I want both Intel & WindRiver engineers could find where the patches are ported from which place at the initial porting stage.<o:p></o:p></span></p>
</div>
</blockquote>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">[dt] The reference should NOT be removed. If a patch is being rebased the reference to the original patch must be preserved.<o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif;color:#1F497D">[Changcheng]: I didn’t get your point. Commit message should be kept there. For the reference(how to find the original patch), I’ll keep them in the initial
porting stage. Once we verified these ported patches works in the end, I’ll remove the reference since they can’t be accessed by external Intel/WindRiver engineers. Do you mean that we should keep “url link” in the commit message directly? Intel internal github
can’t be accessed by external engineers. <o:p></o:p></span></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">
<div>
<div>
<p class="MsoNormal" style="text-indent:12.0pt"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif">3.
<b>[Dean] It also seems like it would be easier to review and merge these in smaller batches.</b><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif"> [Changcheng] Yes. I’m syncing with WindRiver engineers to check whether we could merge some patches firstly to avoid times of rebase and review.<o:p></o:p></span></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">[dt] I am not talking about merging commits, I am talking about splitting the existing commits into multiple Github PRs. Please do not merge commits that are not directly related to each other..<o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif;color:#1F497D">[Changcheng] Some patches are depended on previous patches. We could try to extract patches into new PR later. Currently, we need make sure ceph works locally.
The PR could be accessed by WindRiver engineers who’re working with us to debug the problems.<o:p></o:p></span></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">
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif"> 4.
<b>[Dean] There is also no reference in either the commit messages or the PR description to a Storyboard story or task or any further documentation to why this work is being done.</b><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif"> [Changcheng] I’ll add related information in PR message if we agree with merge part of patches firstly.<o:p></o:p></span></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black">[dt] </span>Please put it into the commit messages. The PR text is not part of the git repo and is lost github is unavailable.<o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif;color:#1F497D">[Changcheng] Yes. Right commit message should be kept there. If you find some commit message is lost, please tell give comment in the patch. Currently, I
have kept all the commit message in the right place. For PR message, we could refine them to meet with your requirement.
<o:p></o:p></span></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">
<div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif"> 5.
<b>[Dean] only the git commit messages are guaranteed to stay with the code changes.</b><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif"> [Changcheng] We’ll give document about stx-ceph upgrade once it’s been upgraded successfully.<o:p></o:p></span></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black">[dt]That </span>does not address the need to put good information into the individual commit messages.<o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif;color:#1F497D">[Changcheng] As I’ve said previously “If you find some commit message is lost, please tell give comment in the patch. Currently, I have kept all the commit
message in the right place.”<o:p></o:p></span></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">
<div>
<div>
<p class="MsoNormal" style="text-indent:12.0pt"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif">6.
<b>[Dean] I had asked for the relevant information to be included in the individual commit messages and I still do not see that being done. We are losing valuable information and traceability for why we are making these changes to upstream.</b><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif"> [Changcheng] Personally, I think I’ve kept most part of original commit message in the new ported patches. Some huge patch is divided into small patches(If you
look the original patch, it’s merged by several patches. It’s hard to be maintained). For PR info, we could give more detail info according to your requirement.<o:p></o:p></span></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black">[dt]Thank </span>you for splitting up previously squashed patches. Please do not confuse PR information (the text that is part of a Github PR) with a commit message (the text that is part of a git commit). Github
PRs are not the place of record for us. Information that does not fit into a git commit message should be in Storyboard or Launchpad, the two places we keep track of those things. But more importantly, things that the next team that looks at this code will
want to have without access to Github or Storyboard or Launchpad needs to be in the commit message. This is exactly the problem we have with the existing patches agains upstream code where we have commitmessages sometimes with only a link to a ticketing system
that we do not have access to. I do not want that to continue.<o:p></o:p></p>
<p class="MsoNormal"><span style="font-size:10.0pt;font-family:"Bookman Old Style",serif;color:#1F497D">[Changcheng] We could avoid much effort if we didn’t squash several patches into one big patch in stx-ceph/v10.2.2. Thanks for your reminding about the distinguish
between PR info and patch commit message. For PR info, we’ll refine them with your requirement. For patch commit message, I haven’t found any serious problem. If you find something wrong, give comment in the patch directly.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">dt<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
<p class="MsoNormal">-- <o:p></o:p></p>
<div>
<p class="MsoNormal"><br>
Dean Troyer<br>
<a href="mailto:dtroyer@gmail.com" target="_blank">dtroyer@gmail.com</a><o:p></o:p></p>
</div>
</div>
</div>
</body>
</html>