User: Password:
Subscribe / Log in / New account

KS2011: Patch review

KS2011: Patch review

Posted Oct 26, 2011 3:32 UTC (Wed) by tytso (subscriber, #9993)
In reply to: KS2011: Patch review by neilbrown
Parent article: KS2011: Patch review

Let's consider when a Signed-off-by: should appear.

(1) when a patch is written by the original author and submitted for consideration. At that point, the author had better be convinced that it is Correct and Sane, since he or she is putting their name and reputation on the line with that patch. In some cases patches are submitted for comments, and in that case sometimes the signed-off-by line is not present (because the author knows some changes might be necessary), or to make the point that the patch really isn't suitable for inclusion, might even have a NOT-Signed-off-by: line to make that point clear.

(2) when the patch is copied from e-mail into a git repository. If the person with the git repository then sends a pull request to Linus, it had better mean that she has put her name and reputation on the line, and is confident that the patch is good. It may not mean that the person has reviewed it; it may be that he is depending on his reviewers or sub-maintainers to have done a good job. But ultimately, it is the maintainer's reputation which is on the line. If the maintainer isn't sure about a sub-maintainer's competence in review and good taste, then she should be doing spot checks or even review every single commit before accepting a pull from the sub-maintainer --- or not stop pulling form the sub-maintainer altogether.

This also implies that in practice, a commit should generally only have two signed-off-by lines. There is an exception, which is if there is a sub-maintainer who isn't using git, so a patch gets forwarded outside of git an additional time. An example of this Andrew Morton's mm tree, which is maintained as a quilt patch series outside of git. But in general, patches are reviewed on e-mail, and once they hit git, they're not going to accumulate any more signed-off-by lines. As those git commits get merged into more senior trees, the maintainer who accepts the pull request is effectively putting their imprimatur on them. So they had better have reviewed the patch, or be willing to put their reputation on the line based on their trust of their sub-maintainers.

(Log in to post comments)

Copyright © 2017, Eklektix, Inc.
Comments and public postings are copyrighted by their creators.
Linux is a registered trademark of Linus Torvalds