On Sat, 18 May 2019, Theodore Ts'o wrote: > On Sat, May 18, 2019 at 07:38:34AM +0100, Lee Jones wrote: > > "- Acked-by: indicates an agreement by another developer (often a > > maintainer of the relevant code) that the patch is appropriate for > > inclusion into the kernel." > > > > And I, as a developer (and not a Maintainer in this case) do indicate > > that this patch is appropriate for inclusion into the kernel. > > > > Reviewed-by has stronger connotations and implies I have in-depth > > knowledge of the subsystem/driver AND agree to the Reviewer's > > Statement. I use Acked-by in this case as a weaker agreement after a > > shallow review of the patch based on its merits alone. > > Note the "often a maintainer of the relevant code" bit. And Exactly, with the *often* (but not always, right!) being the operative word in that sentence. As much as I understand the meaning when used by a Maintainer when commenting on their own subsystem (I use it in this way a lot too), it doesn't always mean "it's okay for you to take this patch which usually comes under my jurisdiction". I think you're missing and if () statement in your understanding: if (maintainer_of_patches'_subsystem) apply_patch_with_supplied_ack(); else /* Where 'n' is the regard you hold for the ack supplier. */ add_n_units_to_patch_credibility(n); > "appropriate for inclusion into the kernel" means to me that you've > done the same level of review as Reviewed-by. So I have very Actually it doesn't, or else the documentation text for Acked-by would be just as intense as it is for Reviewed-by. Reviewed-by IMHO has a much stronger standing than an Acked-by (caveat: when not provided by a maintainer of the appropriate subsystem). > different understanding of how Acked-by and Reviewed-by than you do. Yes, this is seemingly the case. It's apparent that the documentation is not a clear as perhaps it should be, else we wouldn't be having this conversation. Maybe this is something which should be discussed a Kernel Summit. The result being a patch or two which firms up the wording/explanation somewhat. > That being said, no offence to you, but any kind of Acked-by or > Reviewed-by from you is not going to have as much weight as say, a > Reviewed-by: from someone like Jan Kara. Seeing as Jan is a filesystem maintainer, this kind of goes without saying. In fact, the only reason a person might have to take the time to write something like this is to attempt to belittle and cause offence. I could be wrong, but probably not. :) > That's just because I don't have a good sense to your technical > ability I guess you could always use Git to gain a reasonable sense of my technical ability. The 4,000 committed contributions and many more thousands of reviews on the mailing list(s), should give you a brief glimpse. > and so I'd be doing a full review myself I'd think a great deal less of you if you didn't. > and not rely on your review at all.... "at all" - wow! What kind of message do you think this gives to first time contributors (like Philippe here), or would-be reviewers? That there isn't any point in attempting to review patches, since Maintainers are unlikely to take it into consideration "at all"? I know that when I come to review a patch, if *any* contributor has taken the time to review a patch, it always plays an important role. > P.S. And if I find a problem in the patch, and someone had attached > their Acked-by or Reviewed-by to it, it would have the same negative > hit to their reputation either way. Not a big deal if it happens only > once, or it's an esepcially tricky issue, but it if happens more than > once or is really blatent, I as the maintainer definitely do remember. Again, not really sure of your intentions when you write this out, or what it has to do with this patch submission or the review there after, but IMHO this is sending the wrong message to new and would-be contributors. As a community we're supposed to be providing a supportive, encouraging atmosphere. This paragraph is likely to do nothing more than scare off people who would otherwise be willing to have a go [at submitting or reviewing a patch]. -- Lee Jones [李琼斯] Linaro Services Technical Lead Linaro.org │ Open source software for ARM SoCs Follow Linaro: Facebook | Twitter | Blog
