[Patch] contrib/gcc-changelog: Check that PR in subject in in changelog
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,
>>> + 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 == 'Update copyright years.':
>> 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?
> 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...
Size: 4103 bytes
Desc: not available
More information about the Gcc-patches