Re: where is PRnnnn required again?

[Marek Polacek <polacek@redhat.com>]

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, <msebor@gmail.com
> > <mailto:msebor@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 <msebor@redhat.com
> >     <mailto:msebor@redhat.com> <mailto:msebor@redhat.com
> >     <mailto:msebor@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.

> 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?

Marek