Re: [Qemu-devel] [PULL v2 12/27] target/mips: Convert to CPUClass::tlb_fill

[Philippe Mathieu-Daudé]

On 5/14/19 5:48 PM, Alex Bennée wrote:
> 
> Aleksandar Markovic <address@hidden> writes:
> 
>> On May 10, 2019 8:57 PM, "Richard Henderson" <address@hidden>
>> wrote:
>>>
>>
>> Please change the title to 'target/mips: Switch to using
>> mips_cpu_tlb_fill()', or something along that line.
> 
> It does seem a little redundant as "target/mips:" already marks it as a
> mips specific change and viewing the log you can see a series of
> architectures being converted to a new API.
> 
>> Also, the reason for changing the field access_type to mips_access type
>> should be explained in the commit message.
> 
> ok
> 
>> This commit message is generally poor, as it explains relatively
>> unimportant logging issue, while not explaining the core of the
>> change.
> 
> Surely the core of the change is explained in the main patches that
> introduce the new API? I think it would be redundant to repeat that for
> every individual architecture touched. It's a shame it's hard to
> explicitly reference a patch in the same series as the commit hashes are
> not yet permanent. At least when we fix things referring to the short
> hash of the original commit is fairly easy.

Except in the case the maintainer is sending a pull request (like here)
where he can manually fix the commits. Still this is a PITA...

> 
> Generally for an architecture conversion I want to know what might be
> different from other architectures converted. If it is a broadly
> mechanical change it doesn't need to be too detailed.