On 08.12.2022 11:49, George Dunlap wrote: > From: George Dunlap <[email protected]> > > There was a question raised recently about the requriements for > checking in a patch which was originally written by one maintainer, > then picked up and modified by a second maintainer, and which they now both > agree should be checked in. > > It was proposed that in that case, the following set of tags would suffice: > > Signed-off-by: First Author <...> > Signed-off-by: Second Author <...> > Reviewed-by: First Author <...> > > The rationale was as follows: > > 1. The patch will be a mix of code, whose copyright is owned by the > various authors (or the companies they work for). It's important to > keep this information around in the event, for instance, of a license > change or something else requiring knowledge of the copyright owner. > > 2. The Signed-off-by of the Second Author approves not only their own > code, but First Author's code;
The wording in ./MAINTAINERS looks good to me, so it may be benign that here a perhaps relevant (in the course of development) aspect is not expressed correctly: Second Author may have fixed a bug in the original patch, which surely he then doesn't approve. So I'd be inclined to suggest making this "..., but also the unchanged parts of First Author's code". > the Reviewed-by of the First Author > approves not only their own code, but the Second Author's code. Thus > all the code has been approved by a maintainer, as well as someone who > was not the author. > > In support of this, several arguments were put forward: > > * We shouldn't make it harder for maintainers to get their code in > than for non-maintiners > > * The system we set up should not add pointless bureaucracy; nor > discourage collaboration; nor encourage contributors to get around > the rules by dropping important information. (For instance, by > removing the first SoB, so that the patch appears to have been > written entirely by Second Author.) > > Concerns were raised about two maintainers from the same company > colluding to get a patch in from their company; but such maintainers > could already collude, by working on the patch in secret, and posting > it publicly with only a single author's SoB, and having the other > person review it. > > There's also something slightly strange about adding "Reviewed-by" to > code that you've written; but in the end you're reviewing not only the > code itself, but the final arrangement of it. There's no need to > overcomplicate things. > > Encode this in MAINTAINERS as follows: > > * Refine the wording of requirement #2 in the check-in policy; such > that *each change* must have approval from someone other than *the > person who wrote it*. > > * Add a paragraph explicitly stating that the multiple-SoB-approval > system satisfies the requirements, and why. > > Signed-off-by: George Dunlap <[email protected]> Since the actual change to the policy looks good to me: Acked-by: Jan Beulich <[email protected]> Jan
