On 7/7/21 3:53 PM, Marek Polacek wrote:
On Wed, Jul 07, 2021 at 03:35:35PM -0600, Martin Sebor wrote:
On 7/7/21 2:42 PM, Jonathan Wakely wrote:
On Wed, 7 Jul 2021, 17:39 Martin Sebor, <mse...@gmail.com
<mailto:mse...@gmail.com>> wrote:
On 7/6/21 4:09 PM, Jonathan Wakely wrote:
>
>
> On Tue, 6 Jul 2021, 22:45 Martin Sebor via Gcc, <gcc@gcc.gnu.org
<mailto:gcc@gcc.gnu.org>
> <mailto:gcc@gcc.gnu.org <mailto:gcc@gcc.gnu.org>>> wrote:
>
> On 7/6/21 3:36 PM, Marek Polacek wrote:
> > On Tue, Jul 06, 2021 at 03:20:26PM -0600, Martin Sebor via
Gcc wrote:
> >> I came away from the recent discussion of ChangeLogs
requirements
> >> with the impression that the PRnnnn bit should be in the
subject
> >> (first) line and also above the ChangeLog part but
doesn't need
> >> to be repeated again in the ChangeLog entries. But my commit
> >> below was rejected last Friday with the subsequent
error. Adding
> >> PR middle-end/98871 to the ChangeLog entry let me push
the change:
> >>
> >>
https://gcc.gnu.org/g:6feb628a706e86eb3f303aff388c74bdb29e7381
> >>
> >> I just had the same error happen now, again with what
seems like
> >> a valid commit message. Did I misunderstand something or has
> >> something changed recently?
> >>
> >> Martin
> >>
> >> commit 8a6d08bb49c2b9585c2a2adbb3121f6d9347b780 (HEAD ->
master)
> >> Author: Martin Sebor <mse...@redhat.com
<mailto:mse...@redhat.com> <mailto:mse...@redhat.com
<mailto:mse...@redhat.com>>>
> >> Date: Fri Jul 2 16:16:31 2021 -0600
> >>
> >> Improve warning suppression for inlined functions
[PR98512].
> >>
> >> Resolves:
> >> PR middle-end/98871 - Cannot silence
-Wmaybe-uninitialized at
> >> declaration si
> >> te
> >> PR middle-end/98512 - #pragma GCC diagnostic ignored
> ineffective in
> >> conjunct
> >> ion with alias attribute
> >
> > This should be just
> >
> > PR middle-end/98871
> > PR middle-end/98512
> >
> > , no?
>
> Does it matter if there's text after the PR ...?
>
>
>
> Yes. With extra text the whole line is just treated as arbitrary
text,
> not a "PR component/nnnn" string. So with the extra text it won't be
> added to the ChangeLog file, and won't match the PR in the
subject line.
>
> I managed to push
>
> https://gcc.gnu.org/pipermail/gcc-cvs/2021-July/350316.html
>
> that uses the same style earlier today
>
>
> But will it add the PR numbers to the ChangeLog? I think the
answer is
> no (in which case you could edit the ChangeLog tomorrow if you
want them
> to be in there).
It updated Bugzilla but it didn't add the PR numbers to the ChangeLog
entries. I still don't (obviously) understand the rules the hook uses
for what to update or the rationale for them. It seems as though
the PR in the subject is used to update only Bugzilla but not also
update the ChangeLogs (why not?)
Because they are two completely separate processes. Verifying the commit
message format is done by a git hook, and you can run exactly the same
checks locally before pushing a commit.
Updating bugzilla is done by a separate and different process, which has
been in place for years (decades?) before we switched to git.
I don't mean to turn this into a contentious back and forth but
"because this is how it works" or "because this is how it's been
done for eons" aren't a rationale, at least not a satisfying one.
Do you not agree that it would be better to be able to mention
the PR or PRs just once and have all our scripts work with it?
If you do then is something keeping us from making those changes?
Martin
PS To be clear, I'm suggesting that all these work the same and
update Bugzilla as well as ChangeLogs, both with and without
a space after PR and both with and without a component name after
the PR.
1) PR only in title.
Fix foobar [PR12345]
gcc/ChangeLog:
* foo.c (bar): Fix it.
The script would have to derive the component name from the PR number.
That might a complication.
Right, it would have to get from Bugzilla. The mklog.py script
has an option to do that (get both the PR title and component).
2) PR (with or without additional text after it) after title and
before ChageLogs.
Fix foobar.
PR12345 - foobar broken
gcc/ChangeLog:
* foo.c (bar): Fix it.
Looks like the best variant to me (I agree that enabling "- <description>"
after the PR number would be good).
3) PR only in ChangeLogs.
Fix foobar.
gcc/ChangeLog:
PR 12345
* foo.c (bar): Fix it.
I would be really unhappy with this one because I often look for PR numbers
in the GCC mailing list archives and so having those numbers in email subjects
helps tremendously. Therefore, best if people continue putting the #s in
the subject.
I'm not sure why you keep hitting so many issues; git addlog takes care of
this stuff for me and I've had no trouble pushing my patches. Is there
a reason you don't use it also?
I probably have a completely different workflow. Git addlog isn't
a git command (is it some sort of a GCC extension?), and what I put
in the subject of my emails is almost never the same thing as what
I put in the commit message. I'm not suggesting people change their
habits, just that our tooling not unnecessarily penalize those of us
who dot things a little differently.
Martin
