<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Exchange Server">
<!-- converted from rtf -->
<style><!-- .EmailQuote { margin-left: 1pt; padding-left: 4pt; border-left: #800000 2px solid; } --></style>
</head>
<body>
<font face="Bookman Old Style" size="2"><span style="font-size:10pt;">
<div>Hi Dean,</div>
<div> 1. <b>[</b><b>Dean</b><b>]</b><b> </b><b>Notice how the second paragraph of the original message is missing from the new commit.</b></div>
<div> [Changcheng] The second paragraph isn't missed in the new patch.</div>
<div> You could find: " RevertMe: " & " This commit should be reverted when we decide to enable the ceph user."</div>
<div> </div>
<div> 2. <b>[Dean] </b><b>the references to the original commit are not available externally</b></div>
<div> [Changcheng] Yes. The reference should be removed from commit message at last.</div>
<div> Originally, I want both Intel & WindRiver engineers could find where the patches are ported from which place at the initial porting stage.</div>
<div> </div>
<div style="text-indent:12pt;">3. <b>[Dean] </b><b>It also seems like it would be easier to review and merge these in smaller batches.</b></div>
<div> [Changcheng] Yes. I’m syncing with WindRiver engineers to check whether we could merge some patches firstly to avoid times of rebase and review.</div>
<div style="text-indent:12pt;"><font face="Times New Roman"> </font></div>
<div> 4. <b>[Dean] </b><b>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></div>
<div> [Changcheng] I’ll add related information in PR message if we agree with merge part of patches firstly.</div>
<div> </div>
<div> 5. <b>[Dean] </b><b>only the git commit messages are guaranteed to stay with the code changes.</b></div>
<div> [Changcheng] We’ll give document about stx-ceph upgrade once it’s been upgraded successfully.</div>
<div> </div>
<div style="text-indent:12pt;">6. <b>[</b><b>Dean</b><b>]</b><b> </b><b>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></div>
<div> [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.</div>
<div> </div>
<div>B.R.</div>
<div>Changcheng</div>
<div><font face="Times New Roman"> </font></div>
<a name="_MailEndCompose"></a>
<div><img width="83" height="41" src="rtfimage://"></div>
<a name="_____replyseparator"></a>
<div>-----Original Message-----<br>
From: Dean Troyer [<a href="mailto:dtroyer@gmail.com">mailto:dtroyer@gmail.com</a>]
<br>
Sent: Thursday, December 13, 2018 12:20 AM<br>
To: Xie, Cindy <cindy.xie@intel.com><br>
Cc: starlingx-discuss@lists.starlingx.io<br>
Subject: Re: [Starlingx-discuss] Notes: Weekly StarlingX non-OpenStack Distro meeting, 12/12</div>
<div><font face="Times New Roman"> </font></div>
<div>On Wed, Dec 12, 2018 at 8:47 AM Xie, Cindy <<a href="mailto:cindy.xie@intel.com">cindy.xie@intel.com</a>> wrote:</div>
<div>> 3. Ceph upgrade status (Vivian/Dehao/Changcheng) Dean merged all </div>
<div>> the build process patche (on staging stx-ceph and Changcheng finish </div>
<div>> rebasing all patches according to latest stx-ceph (stx/v13.2.2). </div>
<div>> totally 17 PRs New PR: <a href="https://github.com/starlingx-staging/stx-ceph/pull/18">https://github.com/starlingx-staging/stx-ceph/pull/18</a> pending for review.</div>
<div> </div>
<div>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. Let's look at an example:</div>
<div> </div>
<div>In [0] we have the following commit message:</div>
<div> </div>
<div>----------</div>
<div>Port: RevertMe: Use user root to run ceph services</div>
<div> </div>
<div>Avoid debugging file permission issues when upgrading to Jewel.</div>
<div>This is done to provide the same setup as Hammer in StarlingX.</div>
<div>This commit should be reverted when we decide to enable the ceph user.</div>
<div> </div>
<div>Port From:</div>
<div> Ceph Rebase: Disable ceph user/group for Hammer equivalence.patch</div>
<div> 0001____src_ceph-disk_ceph_disk_main.py.patch</div>
<div>0002____src_init-ceph.in.patch</div>
<div>0003____wrs_ceph.conf.patch</div>
<div> </div>
<div>Signed-off-by: Robert Church <<a href="mailto:robert.church@windriver.com">robert.church@windriver.com</a>></div>
<div>Signed-off-by: Daniel Badea <<a href="mailto:daniel.badea@windriver.com">daniel.badea@windriver.com</a>></div>
<div>Signed-off-by: Changcheng Liu <<a href="mailto:changcheng.liu@intel.com">changcheng.liu@intel.com</a>></div>
<div>Signed-off-by: Dehao Shang <<a href="mailto:dehao.shang@intel.com">dehao.shang@intel.com</a>></div>
<div>----------</div>
<div> </div>
<div>This appears to correspond to the original R5 commit c87de31f that has the following commit message:</div>
<div> </div>
<div>----------</div>
<div> Ceph Rebase: Disable ceph user/group for Hammer equivalence</div>
<div> </div>
<div> Use default (root) user to run ceph services instead of dedicated (ceph)</div>
<div> user and group to avoid debugging file permission issues while upgrading</div>
<div> to Jewel.</div>
<div> </div>
<div> This is done to provide the same setup as Hammer in TiS. This commit</div>
<div> should be reverted when we decide to enable the ceph user.</div>
<div>----------</div>
<div> </div>
<div>Notice how the second paragraph of the original message is missing from the new commit.</div>
<div> </div>
<div>Also, the references to the original commit are not available externally, I have no idea what "0001____src_ceph-disk_ceph_disk_main.py.patch" refers to. So even for someone with access to the original commit I have to do text string searches to attempt
to locate it in the R5 repo.</div>
<div> </div>
<div>It also seems like it would be easier to review and merge these in smaller batches. One big PR with 35 commits takes time to review, and when a single change needs to be made we have to re-review looking for the changes.</div>
<div> </div>
<div>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.</div>
<div> </div>
<div>Think of what you have available while doing this rebase/upgrade and imagine what the next person doing the next rebase/upgrade will want to see and make sure all of that is present in the commit messages.</div>
<div>The GitHub PR may or may not be available at that time, only the git commit messages are guaranteed to stay with the code changes.</div>
<div> </div>
<div>dt</div>
<div> </div>
<div> </div>
<div>[0] <a href="https://github.com/starlingx-staging/stx-ceph/pull/18/commits/552736f77f39897922e562a8477d19ab4e47a39f">https://github.com/starlingx-staging/stx-ceph/pull/18/commits/552736f77f39897922e562a8477d19ab4e47a39f</a></div>
<div><font face="Times New Roman"> </font></div>
<div><font face="Times New Roman"> </font></div>
<div>-- </div>
<div> </div>
<div>Dean Troyer</div>
<div><font face="Times New Roman"><a href="mailto:dtroyer@gmail.com"><font face="Bookman Old Style">dtroyer@gmail.com</font></a></font></div>
<div><font face="Times New Roman"> </font></div>
<div>_______________________________________________</div>
<div>Starlingx-discuss mailing list</div>
<div><font face="Times New Roman"><a href="mailto:Starlingx-discuss@lists.starlingx.io"><font face="Bookman Old Style">Starlingx-discuss@lists.starlingx.io</font></a></font></div>
<div><font face="Times New Roman"><a href="http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss"><font face="Bookman Old Style">http://lists.starlingx.io/cgi-bin/mailman/listinfo/starlingx-discuss</font></a></font></div>
<div><font face="Times New Roman"> </font></div>
</span></font>
</body>
</html>