Re: [Patch] contrib/mklog.py: Improve PR handling (was: git gcc-commit-mklog doesn't extract PR number to ChangeLog)

[Iain Sandoe <idsandoe@googlemail.com>]

> On 21 Jun 2021, at 08:26, Martin Liška <mliska@suse.cz> wrote:
> 
> On 6/21/21 8:42 AM, Tobias Burnus wrote:
>> On 18.06.21 16:41, Jason Merrill wrote:
>>> 
>>>     * Being able to specify the PR numbers on the command line in addition
>>>        (currently, they are only extracted from the testsuite patches)
>>> 
>>> 
>>> This bit seems unnecessary to me, since we want the commit to include
>>> tests that identify the PR.
>> I full hearty disagree!
> 
> Hello.
> 
> I do support Tobias' patch and I really think -b argument is handy. Question is,
> if one can use it from 'git commit-mklog' hook? I'm asking because I don't use mklog.py
> directly.

+1 …

and probably including options to the commit .. 
so for something like

git commit-mklog -s -b xxxxxx

or even (if this makes sense )

git commit-mklog —amend -b xxxxx

perhaps we need a way to indicate which sub-process the options are for?

Iain

> 
> Martin
> 
>>> Martin Sebor's patch to extract the PR number from the testcase
>>> filename, as an alternative to a comment, should be enough.
>> Regarding the PR from the filename, are we only talking about
>> new files – or about modifications to old files?
>> For new files, together with -p it seems to be useful; for
>> old files, I think there will be more spurious/false PRs than
>> useful/right PRs.
>> Looking through the last few commits, in all of the following
>> cases the -b would be useful — and I do regard the commits
>> messages are sensible:
>> * cc9c94d43dcfa98436152af9c00f011e9dab25f6 PR libstdc++/100387
>>   added testsuite/25_algorithms/minmax/constrained.cc (etc.)
>>   but file neither has a pr... name nor contains a PR line
>>   (BTW: The fix is not primarily for the PR but fixes it as,
>>    a side effect; hence, not having a PR line makes sense)
>> * 870b674f72d4894b94efa61764fd87ecec29ffde
>>   'ranger' - removes a datastructure (no testcase but also
>>   does not make sense here)
>> * 17a4bee01c3b29c5ccdd39f34384521e5d44135b
>>   Fixes an ICE for one testcase (but of course does not add
>>   a new testcase or modify an existing one.)
>> * 76e990fd211cbb20bf74ce074eb8b2d7b096d3b7
>>   fix a bootstrap issue (found with the GCC 11 backport).
>> Those are all commits between last Friday and today. Hence, I do
>> believe it makes sense to be able to specify the PR on the
>> command line — besides obtaining it from file names.
>> Tobias
>> -----------------
>> Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf
>