[Starlingx-discuss] Notes: Weekly StarlingX non-OpenStack Distro meeting, 12/12

Dean Troyer dtroyer at gmail.com
Wed Dec 12 16:19:42 UTC 2018


On Wed, Dec 12, 2018 at 8:47 AM Xie, Cindy <cindy.xie at 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 at windriver.com>
Signed-off-by: Daniel Badea <daniel.badea at windriver.com>
Signed-off-by: Changcheng Liu <changcheng.liu at intel.com>
Signed-off-by: Dehao Shang <dehao.shang at 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/552736f77f39897922e562a8477d19ab4e47a39f


-- 

Dean Troyer
dtroyer at gmail.com



More information about the Starlingx-discuss mailing list