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

[Mark Harmstone <mark@harmstone.com>]

On 9/1/23 09:22, Christophe Lyon wrote:
>
>
> 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

You both raise good points. Sorry, I still find mailing list culture very confusing, compared to something like GitHub... it's not at all clear what you're expected to do, who has what authority, etc. I'm not at all certain that I was even expected to resubmit the patches the third time, given it was more or less a trivial rebase.

Jan, I suggest that I push patches 1 and 4-7 with the original messages, and save the two test patches for a later date. Would that be acceptable? Patches 2 and 3 probably should have been submitted separately anyway, as the later patches don't depend on them.

Mark