Re: [PATCH 1/7] Fix size of external_reloc for pe-aarch64

[Christophe Lyon <christophe.lyon@arm.com>]

On 1/9/23 09:11, Jan Beulich wrote:
> On 06.01.2023 18:51, Mark Harmstone wrote:
>> On 6/1/23 09:47, Christophe Lyon wrote:
>>> Hi!
>>>
>>> I am not a maintainer, but would you mind adding proper commit messages describing what each patch does (or intends to)?
>>>
>>> Thanks,
>>>
>>> Christophe
>>
>> Hi Christophe,
>>
>> This is a resubmission of a patch set from a few days ago, because of a change that Tamar wanted. If you're interested in the discussion behind each patch, it's available in the mailing list archives.
> 
> I'm afraid pointing to list archives for explanations on patches isn't a good
> approach. Once committed, such links will not be easy to (re-)establish.
> Other projects are quite a bit more demanding towards the content of commit
> messages, but I guess some minimal level of explanation wants to be in the
> average binutils patch as well.
> 

That's what I meant I think, thanks for rephrasing :-)

I can't find guidelines on how to contribute patches for binutils, but 
they are similar to GCC's and GDB's. If you just run 'git log' in a 
binutils clone, you'll see what we mean: in addition to short a summary 
(title), commit messages include a description of what the commit does 
and why this is the right change.

It's great if all your changes are obvious for Nick, but they are not 
for others like me ;-)

For instance this patch 1/7 only says "Fix size of external_reloc for 
pe-aarch64", so why is the removal of 
SWAP_IN_RELOC_OFFSET/SWAP_OUT_RELOC_OFFSET and r_offset needed?

I did check the list archives, so if I'm not mistaken this is third 
iteration of this patch series? (I saw 1/5 and 1/8 in December, then 
this one in January). The first iteration had an introduction message 
which led to a discussion with Jan and Tamar, and I think an updated 
version of that message would help here, when others will have to try to 
understand these patches in whatever future time ;-)

Also as Jan mentioned on your testsuite patches, can you describe why we 
have to skip so many of them? Jan seems to think that they could be 
adjusted to cope with both formats intead.

Thanks,

Christophe

> Jan