Re: [PATCH v2 2/7] dt-bindings: can: rcar_can: document r8a77965 can support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Eugeniu,

On 17/08/18 16:56, Eugeniu Rosca wrote:
> Hello Kieran,
> 
> On Fri, Aug 17, 2018 at 02:44:25PM +0100, Kieran Bingham wrote:
>> Hi Eugeniu
>>
>> Thank you for the patch.
>>
>> On 12/08/18 14:31, Eugeniu Rosca wrote:
>>> Document the support for rcar_can on R8A77965 SoC devices.
>>> Add R8A77965 to the list of SoCs which require the "assigned-clocks" and
>>> "assigned-clock-rates" properties (thanks, Sergei). Rewrap text.
>>
>> I don't think you needed to say you rewrapped the text in the commit log
>> - but it's fine :)
> 
> IMHO "Rewrap text" is pretty much from the same category as "no
> functional change was intended".

Indeed, but in this instance - there was a functional change. You
modified the paragraph.  In fact, mentioning that you have rewrapped the
text, thus implying that you have made no functional change might cause
a reviewer not to look deeper at the actual differences?


> As a reviewer, I would take these
> details in the commit description any day (and sometimes I would NAK a
> patch which lacks these details), since they precisely express the goals
> set by the author and make reviewer's life easier.
> 
> But, of course, preferences vary and therefore I won't elaborate on that
> too much.

If this was a separate hunk, which you had re-wrapped without making a
change to - I would absolutely agree with you here. The 'rewrapping'
should be mentioned in the commit message, but this in relation to a
paragraph which you had modified.

IMO - if you modify a paragraph of text, rewrapping to make sure it fits
the constraints is part of that modification ... but ... yes we are
debating minor details and preferences here ;) - I have no objection to
you mentioning it.

Regards

Kieran

> 
>>
>>> Signed-off-by: Eugeniu Rosca <erosca@xxxxxxxxxxxxxx>
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@xxxxxxxxxxxxxxxx>
>>
> 
> Best regards,
> Eugeniu.
> 




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux