Hello Nathan,
Thanks for starting the discussion on this list.
On Thu, Sep 24, 2020 at 8:57 AM Nathan Cutler <ncutler(a)suse.com> wrote:
Today it came to my attention that not all Ceph developers agree with the
following cherry-picking rule:
"if a commit could not be cherry-picked from master, the commit message must
explain why that was not possible" [1]
[1]
https://github.com/ceph/ceph/blob/master/SubmittingPatches-backports.rst#ch…
Now, I (Nathan) am the one who wrote these rules down, but I'm not their author.
These rules are a codification of a set of best practices I "inherited" from
Loic. Although I hesitate to speak on his behalf, I don't think he's around here
anymore, so I'll just go ahead and present what I think is the rationale for this
particular rule.
In the past, regressions often happened because bugs got fixed directly in
a stable branch, but not in master. Later, after a new major stable release was
split off from master and users upgraded their clusters to it, BOOM the bugs
were back! Of course, nobody initially knew why, but it was clear that the bug
was a regression. Therefore, forensic investigations of the git history were
undertaken to find the answer to the question: "which commit fixed this bug
in N-1 and why is that commit not in N?".
One possible tactic in such an investigation is to find all commits in the
N-1 stable branch (which does not exhibit the bug) that aren't cherry-picks, but
potentially should have been. One of these might be the fix, but which one?
Some bugs have to be fixed directly in a stable branch: they cannot be
cherry-picked from master for any number of valid reasons. So, in our
hypothetical forensic investigation, we are faced with the necessity of
distinguishing these "good" direct bug-fixing commits from "bad" ones
which
should have been cherry-picks, but are not. But how to make that distinction
when the commit messages themselves are silent on the question of why they
aren't cherry picks? That, I believe, is where this rule came from.
Nowadays, it would seem that this type of forensic investigation is rarely
undertaken. BUT let us ask ourselves, could that be because (1) we have these
cherry-picking rules and (2) they are - for the most part - enforced?
From what I've seen myself, we strictly enforce this rule. I agree
with the rationale you've shared above. Any PR against a stable branch
that includes (OR excludes!) commits or fixes not present on master
must explain why.
--
Patrick Donnelly, Ph.D.
He / Him / His
Principal Software Engineer
Red Hat Sunnyvale, CA
GPG: 19F28A586F808C2402351B93C3301A3E258DD79D