<!-- Some styling for better description lists --><style type='text/css'>dt { font-weight: bold;float: left;display:inline;margin-right: 1em} dd { display:block; margin-left: 2em}</style>
***: ChanServ sets mode: +v mchehab
<br> ChanServ sets mode: +v hverkuil
<br> hverkuil has left
pinchartl: <u>mchehab</u>: https://patchwork.linuxtv.org/patch/49770/
<br> oops wrong window
mchehab: could be here :-)
<br> anyway, applied pending pull requests except for one from sailus, with seems to have patches that were commented
pinchartl: :-)
<br> which one ?
mchehab: #49770 makes sense to me
<br> a sensor patch that converts an atomic i2c read/write sequence into non-atomic ones
<br> #0004-media-ov772x-allow-i2c-controllers-without-I2C_FUNC_.patch
pinchartl: ah yes
<br> would you like a pull request for #49770, or can you apply it from patchwork ?
mchehab: I can pick it directly
<br> hmm... on my queue, it is just before/after this patch:
<br> lmml_49830_media_uvcvideo_support_realtek_s_uvc_1_5_device.patch
<br> lmml_49389_media_uvcvideo_support_realtek_s_uvc_1_5_device.patch
<br> lmml_49830_media_uvcvideo_support_realtek_s_uvc_1_5_device.patch
pinchartl: that one could go on too, but it can also be delayed to v4.19
<br> if you want to apply it to v4.18 please use the version I submitted
<br> it cleans up the code, fixes a small issue, and clarifies the commit message
mchehab: This one, right?
<br> <u>Subject</u>: media: uvcvideo: Support realtek's UVC 1.5 device
<br> <u>From</u>: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
<br> X-Patchwork-Id: 49830
<br> <u>Date</u>: Tue, 22 May 2018 23:32:19 +0300
pinchartl: correct
<br> you can drop the conversation from patchwork
mchehab: sure
pinchartl: the patch has also received a few Tested-by tags
<br> two if I'm not mistaken
<br> (I dropped the ones on the original patch as it has changed)
mchehab: patchwork should get those automatically
<br> Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
<br> Tested-by: Ana Guerrero Lopez <ana.guerrero@collabora.com>
<br> yes, it did
pinchartl: nice
mchehab: <u>pinchartl</u>: one detail for further patches...
<br> when you touch somebody's patches, the line where you describe the changes should start with your e-mail, e. g.:
<br> [laurent.pinchart@ideasonboard.com: Factor out code to helper function, update size checks]
pinchartl: ok. it creates long lines, but I can do that
mchehab: yeah. Sometimes, when it is big, what I do is:
<br> [laurent.pinchart@ideasonboard.com: Factor out code to helper function, update size checks]
<br> ops
<br> [laurent.pinchart@ideasonboard.com: Factor
<br> out code to helper function, update size checks]
pinchartl: are there tools that try to parse those lines automatically, or is it purely informative for human consumers ?
mchehab: AFAIKT, purely informative
<br> so, in my case, I would just use mchehab@kernel.org, instead of mchehab+samsung@kernel.org (in order to make my comments shorter)
<br> btw, found the rule... it is at Documentation/process/submitting-patches.rst
<br> If you are a subsystem or branch maintainer, sometimes you need to slightly
<br> modify patches you receive in order to merge them, because the code is not
<br> exactly the same in your tree and the submitters'. If you stick strictly to
<br> rule (c), you should ask the submitter to rediff, but this is a totally
<br> counter-productive waste of time and energy. Rule (b) allows you to adjust
<br> the code, but then it is very impolite to change one submitter's code and
<br> make him endorse your bugs. To solve this problem, it is recommended that
<br> you add a line between the last Signed-off-by header and yours, indicating
<br> the nature of your changes. While there is nothing mandatory about this, it
<br> seems like prepending the description with your mail and/or name, all
<br> enclosed in square brackets, is noticeable enough to make it obvious that
<br> you are responsible for last-minute changes. Example::
<br> Signed-off-by: Random J Developer <random@developer.example.org>
<br> [lucky@maintainer.example.org: struct foo moved from foo.c to foo.h]
<br> Signed-off-by: Lucky K Maintainer <lucky@maintainer.example.org>
<br> it is somewhat outdated...
pinchartl: "While there is nothing mandatory about this, it
<br> seems like prepending the description with your mail and/or name, all
<br> enclosed in square brackets, is noticeable enough to make it obvious that
<br> you are responsible for last-minute changes."
mchehab: the current practice is to have those comments before the SOB line
pinchartl: I've added the comments there, they were just missing the e-mail address
<br> which is already is the SoB line, that's why I've ommitted it
<br> but I can add it in the future
mchehab: without the e-mail, people may not notice who did it
<br> in my specific case, I have scripts to validate SOB lines and other meta-data
<br> it doesn't parse those comments, so it actually move them to be before the SOB&friends metadata
<br> E. g.:
<br> [laurent.pinchart@ideasonboard.com: Factor out code to helper function, update size checks]
<br> <blank line>
<br> <u>Cc</u>: stable@vger.kernel.org
<br> Signed-off-by: ming_qian <ming_qian@realsil.com.cn>
<br> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
<br> Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
<br> Tested-by: Ana Guerrero Lopez <ana.guerrero@collabora.com>
<br> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
<br> (parsing it would be very painful, specially due to multi-lined comments like that
<br> )
<br> so, I really appreciate if you would add your e-mail there
<br> I'm referring to this one (inside one of your git pull requests): 0004-media-ov772x-allow-i2c-controllers-without-I2C_FUNC_.patch
pinchartl: I can, but it would be nice to retain the order, as that's the recommendation from Documentation/process/submitting-patches.rst
<br> "To solve this problem, it is recommended that
<br> you add a line between the last Signed-off-by header and yours, indicating
mchehab: ops, wrong channel
pinchartl: the nature of your changes."
mchehab: yes, as I said, this is outdated. what most maintainers so is to place them before the SOB block
<br> (either with a blank line between them or not)
<br> mixing those comments with SOB are confusing, IMHO
<br> makes harder to read
<br> and what's there can be as important as what's at the description itself
<br> s/so is to/do is to/
pinchartl: it might be a matter of taste, I find it easier to read when they're placed between SoB lines
<br> I think we should stick with what is documented in Documentation/process/submitting-patches.rst
<br> or get Documentation/process/submitting-patches.rst updated
mchehab: yeah, it makes sense to update it
<br> I'll try to remember to submit a patch for it after the merge window
<br> depending on the discussions, I'll try to change my scripts accordingly