On Wed, Dec 12, 2018 at 8:47 AM Xie, Cindy <cindy.xie@intel.com> wrote:
3. Ceph upgrade status (Vivian/Dehao/Changcheng) Dean merged all the build process patche (on staging stx-ceph and Changcheng finish rebasing all patches according to latest stx-ceph (stx/v13.2.2). totally 17 PRs New PR: https://github.com/starlingx-staging/stx-ceph/pull/18 pending for review.
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: In [0] we have the following commit message: ---------- Port: RevertMe: Use user root to run ceph services Avoid debugging file permission issues when upgrading to Jewel. This is done to provide the same setup as Hammer in StarlingX. This commit should be reverted when we decide to enable the ceph user. Port From: Ceph Rebase: Disable ceph user/group for Hammer equivalence.patch 0001____src_ceph-disk_ceph_disk_main.py.patch 0002____src_init-ceph.in.patch 0003____wrs_ceph.conf.patch Signed-off-by: Robert Church <robert.church@windriver.com> Signed-off-by: Daniel Badea <daniel.badea@windriver.com> Signed-off-by: Changcheng Liu <changcheng.liu@intel.com> Signed-off-by: Dehao Shang <dehao.shang@intel.com> ---------- This appears to correspond to the original R5 commit c87de31f that has the following commit message: ---------- Ceph Rebase: Disable ceph user/group for Hammer equivalence Use default (root) user to run ceph services instead of dedicated (ceph) user and group to avoid debugging file permission issues while upgrading to Jewel. This is done to provide the same setup as Hammer in TiS. This commit should be reverted when we decide to enable the ceph user. ---------- Notice how the second paragraph of the original message is missing from the new commit. 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. 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. 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. 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. 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. dt [0] https://github.com/starlingx-staging/stx-ceph/pull/18/commits/552736f77f3989... -- Dean Troyer dtroyer@gmail.com