Re: [PATCH v2 10/42] esp: introduce esp_get_stc()

[Philippe Mathieu-Daudé]

On 2/11/21 8:53 AM, Mark Cave-Ayland wrote:
> On 10/02/2021 22:33, Philippe Mathieu-Daudé wrote:
> 
>> On 2/9/21 8:29 PM, Mark Cave-Ayland wrote:
>>> This simplifies reading the STC register value without having to
>>> manually shift
>>> each individual 8-bit value.
>>
>> If possible repeat the subject so the sentence is easier to understand.
> 
> I've always read commit messages as summary followed detail, so I've
> tended to avoid repetition if the context is obvious from the summary (a
> quick glance through my inbox suggest that quite a few authors also do
> the same).

Not because other do it means it is a good practice ;)

I suppose it depends on personal review workflow and email client used
or gitk layout. In the one I use the commit description is displayed
first, so I have to look elsewhere to prepend the patch subject which
is using another font.
I'm using Thunderbird on Fedora and X1 carbon but had to do some config
change in the default config because the font was too small, unreadable,
then I added some 1.5x zoom factor. It always looked ugly. Apparently
checking that again it seems the Fedora I used was not supporting these
now displays well but now it should work OK, but I'm still using this
old config. Well, my bad. Not in mood to reinstall. Forget my comment
about having "atomic" commit descriptions then.

> 
> Perhaps adding in the word "function" would help readability here, e.g.
> "This function simplifies reading the STC register value..."?

Sounds better to me :)

> 
>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>>> ---
>>>   hw/scsi/esp.c | 15 ++++++++++++---
>>>   1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> 
> ATB,
> 
> Mark.
>