Question regarding upstreaming patches written by multiple people and checkpatch
Hi, I am trying to upstream a patch set that wasn't written by me. The original patch has a lot of warnings and errors reported by checkpatch. Someone else corrected some of those with another patch and I added on a third patch correcting the rest of the warnings and errors that I thought were appropriate to fix. When checkpatch is run on the file that the patches affect, the output looks okay to me, but when I run checkpatch on the patches themselves, the patches still have the same errors and warnings, because I made my changes in a separate patch. So is it okay to have checkpatch errors/warnings for individual patches as long as the patch series as a whole is reasonably checkpatch clean or should I be amending my changes to the original commit and squashing the other cleanup patch into the first patch, so that I am left with one patch that passes checkpatch? If I should be squashing the patches into a single patch, how do I handle the signed-off-by lines as each of the three patches is currently signed off by the respective author of the patch? Obviously I can sign off on the squashed patch for myself. Do I just send it to the mailing list with just my sign off and CC the authors, so that they can sign off on it? From what I've been told, it sounds like CC'ing the authors and having them reply with their sign off is the appropriate way to handle this, but I want to make sure before I start mailing out patches, and this isn't something that I have been able to find an answer to. Cheers, Joseph
On Thu, Feb 18, 2016 at 06:25:46AM -0500, Joseph Bisch wrote:
Hi,
I am trying to upstream a patch set that wasn't written by me. The original patch has a lot of warnings and errors reported by checkpatch. Someone else corrected some of those with another patch and I added on a third patch correcting the rest of the warnings and errors that I thought were appropriate to fix.
When checkpatch is run on the file that the patches affect, the output looks okay to me, but when I run checkpatch on the patches themselves, the patches still have the same errors and warnings, because I made my changes in a separate patch.
So is it okay to have checkpatch errors/warnings for individual patches as long as the patch series as a whole is reasonably checkpatch clean
Usually, yes, this is fine. But it can depend on the subsystem / area of the kernel you are submitting this for, what type of code is this? What maintainer(s) are you going to have to send the patches to?
or should I be amending my changes to the original commit and squashing the other cleanup patch into the first patch, so that I am left with one patch that passes checkpatch?
I would preserve original authorship and signed-off-by as much as possible. Hope this helps, greg k-h
On Thu, Feb 18, 2016 at 1:46 PM, Greg KH <greg@kroah.com> wrote:
On Thu, Feb 18, 2016 at 06:25:46AM -0500, Joseph Bisch wrote:
Hi,
I am trying to upstream a patch set that wasn't written by me. The original patch has a lot of warnings and errors reported by checkpatch. Someone else corrected some of those with another patch and I added on a third patch correcting the rest of the warnings and errors that I thought were appropriate to fix.
When checkpatch is run on the file that the patches affect, the output looks okay to me, but when I run checkpatch on the patches themselves, the patches still have the same errors and warnings, because I made my changes in a separate patch.
So is it okay to have checkpatch errors/warnings for individual patches as long as the patch series as a whole is reasonably checkpatch clean
Usually, yes, this is fine.
But it can depend on the subsystem / area of the kernel you are submitting this for, what type of code is this? What maintainer(s) are you going to have to send the patches to?
The patch set[0] (link doesn't include my patch) is a driver for the quadrature encoder pulse decoder unit on the AM33xx series SoC. According to get_maintainer.pl, I am sending the patches to OMAP DEVICE TREE SUPPORT maintainers, OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS maintainers, the ARM PORT maintainer, you and the other CHAR and MISC DRIVERS supporter, and the mailing lists.
or should I be amending my changes to the original commit and squashing the other cleanup patch into the first patch, so that I am left with one patch that passes checkpatch?
I would preserve original authorship and signed-off-by as much as possible.
Okay. The author of the second patch said I should squash it into the first (because it was just an upstream kernel interface change), so I put the commit message from the second patch (because it was just one line) in brackets between the two sign off lines as I've seen maintainers do when they need to make a last minute change before accepting a patch. Should I have done it that way? Or would you still keep them as separate patches?
Hope this helps,
Yes it does. Thanks! [0] - https://github.com/RobertCNelson/linux-dev/tree/master/patches/beaglebone/eq... Joseph
participants (2)
-
Greg KH -
Joseph Bisch