[Patch] contrib/gcc-changelog: Check that PR in subject in in changelog

Martin Liška mliska@suse.cz
Thu Jun 10 14:24:04 GMT 2021


On 6/10/21 4:07 PM, Tobias Burnus wrote:
> (Moved to gcc-patches, missed this when I replied to the initial email)
> 
> Regarding patch at: https://gcc.gnu.org/pipermail/gcc/2021-June/236357.html
> 
> On 10.06.21 14:45, Jonathan Wakely wrote:
> 
>> As well as the "contrig" typo that Florian noticed, the subject says
>> "in in" which should be "is in". And it should be CC'd to gcc-patches.
>>
>> I like this more than my attempt, however ...
>>> --- a/contrib/gcc-changelog/git_repository.py
>>> +++ b/contrib/gcc-changelog/git_repository.py
>>> @@ -59,8 +59,9 @@ def parse_git_revisions(repo_path, revisions, ref_name=None):
>>>
>>>              date = datetime.utcfromtimestamp(c.committed_date)
>>>              author = '%s  <%s>' % (c.author.name, c.author.email)
>>> -            git_info = GitInfo(c.hexsha, date, author,
>>> -                               c.message.split('\n'), modified_files)
>>> +            message = c.message.split('\n')
>>> +            git_info = GitInfo(c.hexsha, date, author, message[0],
>>> +                               message[1:], modified_files)
>> Doesn't using message[1:] here mean that other checks which currently
>> look at all of self.info.lines will no longer check the subject line?
> ...
>> For example, we have:
>>          # Skip Update copyright years commits
>>          if self.info.lines and self.info.lines[0] == 'Update copyright years.':
>>              return
>> This will never match now, because you've extracted that into the
>> 'subject' instead.
> 
> Well, it never matched before for git_email.py, it only did match for
> git_repository.py. I think the difference between your work and mine was
> that I started with git_email.py – and you started with git_repository.py.
> 
> I now pass again the whole message to git_commit.py – also for emails. I
> think that's more consistent when checking for an empty line as second line.

I see this approach better.

> 
> And for the copyright case, I added a testcase :-)
> 
>> Aside: We should also have a check that the second line is blank, i.e.
>> the commit message is a single line subject, followed by blank,
>> followed by the body. And if we enforced that, then message[2:] would
>> be better than message[1:].
> Added as check – but I pass now all (also subject + '\n' + body) for the
> email, which I think it easier to grasp. But as three is always an empty
> line for emails (to separate header and body), I cannot add a testcase
> for it, unfortunately.
> 
> Updated patch enclosed.

I'm sending a small update that handles some flake8 issues and as defined in setup.cfg,
line limit for the file is 120 characters.

I have one question: Do we really need the revert regex? What about using GitCommit::revert_commit?

Cheers,
Martin

> 
> Tobias
> 
> -----------------
> Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf

-------------- next part --------------
A non-text attachment was scrubbed...
Name: changelog.diff
Type: text/x-patch
Size: 4103 bytes
Desc: not available
URL: <https://gcc.gnu.org/pipermail/gcc-patches/attachments/20210610/7b7337f4/attachment.bin>


More information about the Gcc-patches mailing list