Messages in this thread | | | Date | Mon, 19 Jul 2021 14:05:23 +0200 | From | Pavel Machek <> | Subject | Re: [PATCH 1/2] swap led_brightness from enum to typedef |
| |
Hi!
> >>> vim +18 include/media/v4l2-flash-led-class.h > >>> > >>> 14 > >>> 15 struct led_classdev_flash; > >>> 16 struct led_classdev; > >>> 17 struct v4l2_flash; > >>> > 18 led_brightness; > >>> 19 > >>> > >>> --- > > > > > Another patch was sent into the list to correct this error. > > Hopefully Pavel (LED subsystem maintainer) will comment soon-ish. > > My comments: > > a. This patch would be the right thing to do if your large patch had already been > applied (merged) somewhere, but AFAIK it hasn't been. So: > > b. IMO you should resend your entire patch set with this fix included. > Send it as "v2" (version 2) and explain the changes in it since your > original patch (which was v1). This v2 explanation should be below the > "---" line in the patch. (See Documentation/process/submitting-patches.rst > for more info -- or ask for more info/help.)
I still remember the old patch, so b. is not strictly neccessary here.
> c. For your follow-up patch to include/media/v4l2-flash-led-class.h, which was: > > -led_brightness; > +typedef u8 led_brightness; > > I would just add this to include/media/v4l2-flash-led-class.h: > > #include <linux/leds.h> > > That way, in a few years, when the type of led_brightness changes again, > someone won't have to remember to search for other typedefs of it and > update them also. Or maybe they will do that after a bug happens and > someone notices it. > > (Note that I am just trying to help. Pavel has more of a final > say-so about this.)
And your comments are reasonable.
Best regards, Pavel -- http://www.livejournal.com/~pavelmachek [unhandled content-type:application/pgp-signature] | |