Re: [PATCH v2] i2c: busses: remove duplicate dev_err()

Re: [PATCH v2] i2c: busses: remove duplicate dev_err()

[Markus Elfring]

> it will print an error message by itself when platform_get_irq()
> goes wrong. so don't need dev_err() in here again.

I suggest to improve the commit message considerably.

Would you like to integrate any information from a response
on the topic “drm/meson: Delete an error message in meson_dw_hdmi_bind()” by
a contributor like Neil Armstrong?
https://lore.kernel.org/dri-devel/54e0b6e4-4a39-c567-7355-2b2330a95294@baylibre.com/
https://lore.kernel.org/patchwork/comment/1418937/
https://lkml.org/lkml/2020/4/15/186


> v1 -> v2:

Will a version identification be sufficient without the arrow?


> 	- add 19 similar changes by Markus's suggestion.

* With which search tool did you find the presented update candidates?

* Will a link to the commit “driver core: platform: Add an error message
  to platform_get_irq*()” (by Stephen Boyd from 2019-07-29) become helpful
  as background information for such collateral evolution?
  https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/base/platform.c?id=7723f4c5ecdb8d832f049f8483beb0d1081cedf6

Regards,
Markus

Re: [PATCH v2] i2c: busses: remove duplicate dev_err()

[Joe Perches]

On Thu, 2020-04-16 at 20:22 +0200, Markus Elfring wrote:
> > it will print an error message by itself when platform_get_irq()
> > goes wrong. so don't need dev_err() in here again.
> 
> I suggest to improve the commit message considerably.

I suggest you ignore Markus' suggestion.

The commit message is OK.

The subject _could_ have included "platform_get_irq" something like

Subject: [PATCH v2] i2c: busses: Remove platform_get_irq()'s duplicated dev_err()

but I believe it's not important enough to redo.

Re: [v2] i2c: busses: remove duplicate dev_err()

[Markus Elfring]

>>> it will print an error message by itself when platform_get_irq()
>>> goes wrong. so don't need dev_err() in here again.
>>
>> I suggest to improve the commit message considerably.
>
> I suggest you ignore Markus' suggestion.
>
> The commit message is OK.

I would appreciate if such a view can be reconsidered.

I got used to looking at information which can be provided there.
Thus I occasionally notice further change possibilities
also at this place.

Examples:
* Should the first word in English sentences start with a capital letter?

* Which value do you give to an imperative wording in the change description?

* Will an other word order become helpful here?

* Can it be relevant to refer to questionable error messages
  instead of the mentioned function name?


> The subject _could_ have included "platform_get_irq" something like
>
> Subject: [PATCH v2] i2c: busses: Remove platform_get_irq()'s duplicated dev_err()

I find such a suggestion for a patch subject also a bit nicer.


> but I believe it's not important enough to redo.

Will your idea be picked up for the final commit title?

Regards,
Markus

Re: [PATCH v2] i2c: busses: remove duplicate dev_err()

[Andy Shevchenko]

On Fri, Apr 17, 2020 at 12:28 AM Joe Perches <joe@perches.com> wrote:
>
> On Thu, 2020-04-16 at 20:22 +0200, Markus Elfring wrote:
> > > it will print an error message by itself when platform_get_irq()
> > > goes wrong. so don't need dev_err() in here again.
> >
> > I suggest to improve the commit message considerably.
>
> I suggest you ignore Markus' suggestion.
>
> The commit message is OK.
>
> The subject _could_ have included "platform_get_irq" something like
>
> Subject: [PATCH v2] i2c: busses: Remove platform_get_irq()'s duplicated dev_err()
>
> but I believe it's not important enough to redo.

+1, I think it's good enough.

-- 
With Best Regards,
Andy Shevchenko