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