| Message ID | efe9964f-c907-305c-b75e-32a4139d1edd@users.sourceforge.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 15 August 2016 at 13:12, SF Markus Elfring <elfring@users.sourceforge.net> wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > Date: Mon, 15 Aug 2016 13:03:29 +0200 > > The field "owner" is set by core. Thus delete an extra initialisation. Hi, Just a small nit on the patch title: "delete owner assignment" is virtually useless as a title because it has no meaning without the broader context and only describes the literal change. It's like naming a patch "add a line" or "change the code"; it serves no purpose. How about "backlight-tosa: delete _unnecessary_ assignment"? This immediately communicates the reason for/intent of the patch (there is unnecessary code, thus we can simplify it). (Sorry about singling out this patch and the apparent bikeshedding, this comment obviously applies to a lot of patches by a lot of authors!) Thanks, Vegard -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> The field "owner" is set by core. Thus delete an extra initialisation. > > Just a small nit on the patch title: "delete owner assignment" is > virtually useless as a title because it has no meaning without the > broader context and only describes the literal change. It's like > naming a patch "add a line" or "change the code"; > it serves no purpose. I have got an other impression. Do you want that I add any more background information to the commit message? > How about "backlight-tosa: delete _unnecessary_ assignment"? Will the underlined key word trigger any related software development concerns? Would another look be needed on how the usage of the mentioned data structure element was reduced over time? Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 15 August 2016 at 15:25, SF Markus Elfring <elfring@users.sourceforge.net> wrote: >>> The field "owner" is set by core. Thus delete an extra initialisation. >> >> Just a small nit on the patch title: "delete owner assignment" is >> virtually useless as a title because it has no meaning without the >> broader context and only describes the literal change. It's like >> naming a patch "add a line" or "change the code"; >> it serves no purpose. > > I have got an other impression. > > Do you want that I add any more background information to the > commit message? No, the rest of the commit message is fine. I was only concerned about the patch title (the first line) since that's what appears frequently in patch lists (cgit, shortlogs, email/archives), etc. >> How about "backlight-tosa: delete _unnecessary_ assignment"? > > Will the underlined key word trigger any related software > development concerns? No, the emphasis was just for the email, I wouldn't put that in the actual commit log. > Would another look be needed on how the usage of the mentioned data > structure element was reduced over time? No, it's fine, it's really just about the patch title :-) Thanks, Vegard -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 15 Aug 2016, Vegard Nossum wrote: > On 15 August 2016 at 13:12, SF Markus Elfring > <elfring@users.sourceforge.net> wrote: > > From: Markus Elfring <elfring@users.sourceforge.net> > > Date: Mon, 15 Aug 2016 13:03:29 +0200 > > > > The field "owner" is set by core. Thus delete an extra initialisation. > > Hi, > > Just a small nit on the patch title: "delete owner assignment" is > virtually useless as a title because it has no meaning without the > broader context and only describes the literal change. It's like > naming a patch "add a line" or "change the code"; it serves no > purpose. > > How about "backlight-tosa: delete _unnecessary_ assignment"? This > immediately communicates the reason for/intent of the patch (there is > unnecessary code, thus we can simplify it). backlight-tosa: Do not manually assign THIS_MODULE to .owner This is unnecessary because ... > (Sorry about singling out this patch and the apparent bikeshedding, > this comment obviously applies to a lot of patches by a lot of > authors!) > > Thanks, > > > Vegard
diff --git a/drivers/video/backlight/tosa_bl.c b/drivers/video/backlight/tosa_bl.c index 83742d8..9706759 100644 --- a/drivers/video/backlight/tosa_bl.c +++ b/drivers/video/backlight/tosa_bl.c @@ -163,7 +163,6 @@ MODULE_DEVICE_TABLE(i2c, tosa_bl_id); static struct i2c_driver tosa_bl_driver = { .driver = { .name = "tosa-bl", - .owner = THIS_MODULE, .pm = &tosa_bl_pm_ops, }, .probe = tosa_bl_probe,