On Wed, Dec 12, 2018 at 8:06 PM Liu, Changcheng <changcheng.liu@intel.com> wrote:
2. *[Dean] **the references to the original commit are not available externally* [Changcheng] Yes. The reference should be removed from commit message at last. Originally, I want both Intel & WindRiver engineers could find where the patches are ported from which place at the initial porting stage.
[dt] The reference should NOT be removed. If a patch is being rebased the reference to the original patch must be preserved.
3. *[Dean] **It also seems like it would be easier to review and merge these in smaller batches.* [Changcheng] Yes. I’m syncing with WindRiver engineers to check whether we could merge some patches firstly to avoid times of rebase and review.
[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..
4. *[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.* [Changcheng] I’ll add related information in PR message if we agree with merge part of patches firstly.
Please put it into the commit messages. The PR text is not part of the git repo and is lost github is unavailable.
5. *[Dean] **only the git commit messages are guaranteed to stay with the code changes.* [Changcheng] We’ll give document about stx-ceph upgrade once it’s been upgraded successfully.
That does not address the need to put good information into the individual commit messages.
6. *[**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.* [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.
Thank 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. dt -- Dean Troyer dtroyer@gmail.com