[PATCH v3 0/3] gas: add command line option to inhibit info messages

[Jan Beulich]

On 10.07.2024 14:34, Richard Earnshaw (lists) wrote:
> On 10/07/2024 13:17, Jan Beulich wrote:
>> On 10.07.2024 13:26, Matthieu Longo wrote:
>>> Hi,
>>>
>>> GAS currently emits information messages for context information along warnings. These extra messages might sometimes pollute the test expectations when checking for error message patterns in stderr output.
>>>
>>> This patch aims at providing two new flags while preserving the existing behavior if none of the options is provided.
>>>   * --info, similar to the existing --warn flag to enable diagnostic information messages (default behavior).
>>>   * --no-info, similar to the existing --no-warn flag to disable diagnostic information messages.
>>>
>>> It also adds the flags to the existing documentation, and command manual.
>>>
>>> As an example of its benefit, I used the new flag for the system register tests under gas/testsuite/gas/aarch64/sysreg testsuite, and the info message for macro expansion were removed inside the tests expectations.
>>>
>>> Regression tested on aarch64-none-elf, and no regression found.
>>>
>>> Ok for binutils-master?
>>
>> You keep re-sending patch 1 with this question, when I gave an okay
>> there already. Yet I don't think there's a strong need for all parts
>> of this series to go in together.
> 
> There are some etiquette and workflow issues to discuss here.  Firstly, it can be helpful to the developer to keep a patch series together, even if parts of it have been approved.  This can help with tools like the Linaro pre-commit CI, which captures results in our patchwork server (https://patchwork.sourceware.org/project/binutils/list/).

I'm having trouble seeing how the committing of an initial part of a
series could cause problems there. Then again I know nothing about that
infrastructure, so I'm almost certainly missing something pretty
important. I can only say that I find it unhelpful if patch volume to
post isn't reduced as much as possible. Every saved mail is a win imo.

>  But we should also make it clearer to reviewers in the cover letter what has changed in each iteration so that reviewers don't need to waste time going back over unchanged material.  The convention in the linux world is that when a patch set is reposted, the cover letter is updated with a summary of the changes since the last iteration.  Something like
> 
> Changes in v3 - update patch 2/3 to fix garbled message
> Changes in v2 
>  - fix tyop in patch 1/3
>  - update patch 3/3 to deal with 48-hour days
> 
> (yes, I know the above are nonsense :) ).

At least partially in Matthieu's defense: He has such a section in the
cover letter, just not split by patch. Personally, doing a lot of reviews
in particular in the Xen Project, I don't like that much either. Best aid
for reviewers is, imo, a per-patch revision log, as then you don't need
to turn back to the cover letter all the time. It may not matter much when
there are few changes, but the more there are the more noticeable the
difference.

Jan