Thread (31 messages) 31 messages, 5 authors, 2021-06-04

Re: [PATCH 02/15] leds: leds-gpio-register: Supply description for param 'id'

From: Uwe Kleine-König <hidden>
Date: 2021-06-02 08:24:44
Also in: lkml

Hello Lee,

On Tue, Jun 01, 2021 at 10:05:03AM +0100, Lee Jones wrote:
On Fri, 28 May 2021, Uwe Kleine-König wrote:
quoted
On Fri, May 28, 2021 at 10:55:31AM +0100, Lee Jones wrote:
quoted
On Fri, 28 May 2021, Uwe Kleine-König wrote:
quoted
On Fri, May 28, 2021 at 10:06:16AM +0100, Lee Jones wrote:
quoted
diff --git a/drivers/leds/leds-gpio-register.c b/drivers/leds/leds-gpio-register.c
index b9187e71e0cf2..de3f12c2b80d7 100644
--- a/drivers/leds/leds-gpio-register.c
+++ b/drivers/leds/leds-gpio-register.c
@@ -11,6 +11,7 @@
 /**
  * gpio_led_register_device - register a gpio-led device
  * @pdata: the platform data used for the new device
+ * @id: platform ID
  *
Given that id is the first parameter and pdata the second I suggest to
swap the order here and describe id first.
That's super picky.

I can do it as a follow-up patch if you *really* care about it.
I'd say introducing the one-line description for id now in the "wrong"
location and then reordering as a followup is ridiculus. But having said
that: I don't care at all.
It's only "wrong" according to you.

I see these presented in a different order to their counterparts all
the time.
This is a poor justification. In software bugs happen all the time, this
doesn't mean you shouldn't make it better when you touch some code.
I do however appreciate that it does make more sense and
is easier on the eye, which is why I am more than happy to rectify.
... still more if you agree that the feedback makes sense.
With regards to the follow-up scenario, it makes far less sense for an
already merged patch in a history tree to be reverted, or for history
to be unnecessarily re-written for something as trivial as this.
Ah, that the patch is already merged is news to me. Indeed, then fixing
this is not sensible. My initial feedback was less than an hour after
you sent the patch and it appeared just yesterday in next, so this
wasn't easily noticeable for me.

Usually I'm annoyed about maintainers who don't react to patch series
and don't apply it. Here I'm more annoyed that I was Cc:d---which I
interpret as a request for feedback---and an hour later was already too
late for my review reply and there was (up to today) no maintainer mail
that the patch set was applied.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help