On Thu, 10 Jun 2021 at 15:56, Martin Sebor wrote: > > On 6/10/21 4:40 AM, Jonathan Wakely via Gcc wrote: > > On Thu, 10 Jun 2021 at 11:08, Jakub Jelinek <ja...@redhat.com> wrote: > >> > >> On Thu, Jun 10, 2021 at 11:01:49AM +0100, Jonathan Wakely via Gcc wrote: > >>> On 10/06/21 10:44 +0100, Jonathan Wakely wrote: > >>>>> Quite interesting idea! Are you willing to prepare a patch for it? > >>>> > >>>> This works. > >>> > >>> And this works better, because it checks the PR in the title matches > >>> one in the changelog. > >>> > >>> I'll get something added to the tests and prep this for commit. > >> > >> Note, some commits fix more than one PR. Sometimes the subject lists > >> just one of them and the ChangeLog several, at other times people mention > >> [PRnnnnnn, PRnnnnnn] etc. in the subject. > >> I think checking that at least one changeLog PR line matches at least one > >> PR > >> in the subject would be good enough. > >> > >> Your regex will not match [PR123456, PR123457] in subject, perhaps ok > > > > Yeah, that wouldn't get matched, so no checks would be done for the > > changelog body. Not ideal, but better than what we have no where > > nothing is checked at all. > > > >> initially, and if I read it will will be happy if at least one line matches > >> it. > > > > Yes, if the summary line has a single PR number, it must be present in > > the changelog body. Other PR numbers can also be in the body, and they > > aren't checked. > > > > But I've hit an issue trying to test it, because the testcases in > > contrib/gcc-changelog/test_patches.txt are in the form of emails, and > > the Subject: line from the emails is not passed to the GitInfo > > constructor, so isn't part of the message that gets checked. > > > > Martin, Shouldn't the GitEmail class extract the Subject: from the > > email header and use that as the first line passed to the GitInfo > > object? > > > > I'm a little lost as to what's being changed,
Nothing is being changed. If your patch is related to a bug, you're supposed to say so: https://gcc.gnu.org/codingconventions.html#ChangeLogs And you're also supposed to put a [PRnnnn] tag in the email subject line (which best practices say should also be the Subject: of the email you send to gcc-patches): https://gcc.gnu.org/contribute.html#patches The patches being proposed verify that if you put [PRnnnn] at the end of the subject line, that you also put "PR component/nnnn" in the changelog part. Because if you didn't, something is wrong and you're failing to follow the existing policy (why would you put a [PRnnnn] tag in the subject if it's not related to that PR, and if it is related, why are you not putting it in the changelog part?) > and, truth be told, > what exactly the current "right" format is. Where are the PRnnnnn > strings recognized as special? > > The ChangeLog description doesn't seem to cover this and I've been > assuming they're recognized anywhere in the ChangeLog message, but > I think I also noticed they don't always end up updating all > the bugs. There are various reasons for that, including that non-ASCII characters tend to break the automation. But in general, if you follow the policy of mentioning "PR component/nnnn" in the changelog, your git commit should add a comment to the mentioned bugzilla PRs. Within a bugzilla comment, any string mentioning "PR nnnn" or "Bug nnnn" (and variations on those) will get dynamically turned into a link to that bug, but I think that happens on the fly when rendering the HTML, and it's separate from the automation that adds git commits to bugzilla as comments. I think the automation to add commits to bugzilla uses a much stricter pattern to recognise a PR mentioned in the commit. > > FWIW, in commits for multiple PRs I've been adding a Resolves > line like this: > https://gcc.gnu.org/pipermail/gcc-cvs/2021-May/347414.html > > I usually also add the PR numbers under each ChangeLog but I'm not > sure it's necessary. It would be good to know and for the ChangeLog > convention to document how exactly this works, and if something > changes, to update the documentation. It already says you should put it in the changelog. For each changelog piece you add it to, it will be added to that relevant ChangeLog file. It probably makes sense to add it to all of them, unless some piece of the commit really isn't relevant to that PR (but then it should probably be a separate commit!) You can also put it once, before all the changelog entries e.g. https://gcc.gnu.org/g:91349e57bbfd010156b9128b2ad751c8843e7245 which got added to two ChangeLog files by the daily bump script: https://gcc.gnu.org/git/?p=gcc.git;a=blobdiff;f=gcc/cp/ChangeLog;h=5a97fc84264e85b60adfdc50187ce7faeeb6f86f;hp=225b891700e88a58639d7ea0f10ad76ffb8d87f4;hb=c60387214593445d1514bf7852f27f4523458cda;hpb=25e5ecdf82b49977e86bfaded236fb34af2705ed https://gcc.gnu.org/git/?p=gcc.git;a=blobdiff;f=gcc/testsuite/ChangeLog;h=9e31d686e1cb3b63b2141f86e5394aa1794ecdf5;hp=640fcbed0ebb9ed5e0b0ab12c52e1950284f9ee9;hb=4f625f47b4456e5c05a025fca4d072831e59126c;hpb=53cb324cb4f9475d4eabcd9f5a858c5edaacc0cf I do want to improve the https://gcc.gnu.org/contribute.html#patches and https://gcc.gnu.org/codingconventions.html#ChangeLogs docs though. We shouldn't have the info split in two places, and we should update the recommendations to reflect current practices.
