From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-io1-xd44.google.com (mail-io1-xd44.google.com [IPv6:2607:f8b0:4864:20::d44]) by sourceware.org (Postfix) with ESMTPS id 937AA388A82A; Tue, 9 Jun 2020 20:29:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 937AA388A82A Received: by mail-io1-xd44.google.com with SMTP id t9so2513955ioj.13; Tue, 09 Jun 2020 13:29:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=/NZ55x8GrJf+I5PVRcQbEJzY4dFY66AsZlfMxRuYISc=; b=FLqpDVmq1gARMseqkayCOHUnia5nAkhbM+e1nQeBrQoKxCzVTQ6YQON1y8XaUQ/fhg opY1Pa2DReNGz1APOLKYxP2MrQ0THfwU43EMXyUalou8jsWDW7YQyBqhe4Pwqn6jVnqh Imoe8glyTTALRx4m/zdhUcoeat7TlpQe2cwJTjxMDkdctQHTCz5eVTIlxerYWak8MJqS 1OeQz4kMIcvewDJfNaarkpf49zz5c8KeFZDfFtECjK6MEg9eR26RU5H+n+7gpdEYKQ3v 3EMu2hR+7G0E0MOUN3HJJoPb0zbYvVE7ENQODguQBLRorB/BW20lY4qXfTopiXLDIc9J APAw== X-Gm-Message-State: AOAM532WmwjZPVZALuNQTuSsHOSrjhJdumVNASmU3TReiSQWpcn2Wsv+ LtxqdcgBts9w03Y5W8IirMoUa+IimTzPJZzXQ28= X-Google-Smtp-Source: ABdhPJwwrJuJfV7sc2ww6SLYcfhLzyugj7/6CFHUIFlRR60iFR3pHH7KD7SLF0W4QSNNBJQIs/7vRdWFNO01yCnY/+Y= X-Received: by 2002:a6b:c9cd:: with SMTP id z196mr28850616iof.172.1591734587004; Tue, 09 Jun 2020 13:29:47 -0700 (PDT) MIME-Version: 1.0 References: <20200525224858.GO8462@tucnak> <58fa71e3-cbc1-7bbd-47ef-fd4f226fc7a5@suse.cz> <3c3ce66f-0e51-25a4-7d5c-745fa3a9b0ab@suse.cz> In-Reply-To: <3c3ce66f-0e51-25a4-7d5c-745fa3a9b0ab@suse.cz> From: Jonathan Wakely Date: Tue, 9 Jun 2020 21:29:35 +0100 Message-ID: Subject: Re: [IMPORTANT] ChangeLog related changes To: =?UTF-8?Q?Martin_Li=C5=A1ka?= Cc: Gerald Pfeifer , Jakub Jelinek , "gcc@gcc.gnu.org" , gcc-patches Content-Type: multipart/mixed; boundary="000000000000a2d7cd05a7ac96a6" X-Spam-Status: No, score=-7.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Jun 2020 20:29:49 -0000 --000000000000a2d7cd05a7ac96a6 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, 2 Jun 2020 at 15:25, Martin Li=C5=A1ka wrote: > > On 6/2/20 4:14 PM, Jonathan Wakely wrote: > > On Tue, 2 Jun 2020 at 14:56, Jonathan Wakely wr= ote: > >> > >> On Tue, 2 Jun 2020 at 14:16, Martin Li=C5=A1ka wrote: > >>> > >>> On 6/2/20 1:48 PM, Martin Li=C5=A1ka wrote: > >>>> I tend to this approach. Let me prepare a patch candidate for it. > >>> > >>> There's a patch for it. Can you please Jonathan take a look? > >> > >> Looks great, thanks! > >> > >> + if name not in wildcard_prefixes: > >> + msg =3D 'unsupported wilcard prefix' > >> > >> Typo "wilcard" > >> > >> + if pattern not in used_patterns: > >> + self.errors.append(Error('a file pattern not used in = a patch', > >> + pattern)) > >> > >> If the script printed this error I don't think I'd know what it was > >> complaining about. How about "pattern doesn't match any changed files" > >> ? > >> > >> I find the existing error 'file not changed in a patch' to be > >> suboptimal too. We're checking revisions (or commits), not patches. > > > > Along those lines, here's an incomplete patch (tests aren't updated > > yet, no commit log, your latest change isn't merged ) to revise the > > error messages. I've tried to make them more consistent (e.g change > > 'file not changed in a patch' to 'unchanged file mentioned in a > > ChangeLog' which mirrors the error for the opposite condition, > > 'changed file not mentioned in a ChangeLog'). > > I welcome this. > > > > > I've also moved line numbers to the start of the line (like GCC's own > > diagnostics) and not used the "line" argument of the Error constructor > > for things that aren't line numbers. I've aimed to be consistent, so > > that line numbers come at the start, pathnames and patterns come at > > the end (and there's a space before them). > > Well, the Error ctor argument 'line' is bit misleading. It's a string and > not an integer: > > File "/home/marxin/Programming/gcc/contrib/gcc-changelog/git_commit.py= ", line 177, in __repr__ > return '%d: %s' % (self.line, self.message) > TypeError: %d format: a number is required, not str > > and it represents a line from a git commit message. I use it in order to = provide > line which is affected by an error. > > > > > I also suggest changing 'additional author must prepend with tab and 4 > > spaces' to 'additional author must be indented with one tab and four > > spaces'.> > > The intent is to improve the user experience, since for many people > > who are new to git *any* error can cause confusion, so extra, > > gcc-specific errors that we issue should aim to be easy to understand. > > I like your wordings. OK, here's a proper patch for the changes you liked, dropping the changes to the Error exception type. pytest contrib/gcc-changelog/test_email.py passes. OK for master? --000000000000a2d7cd05a7ac96a6 Content-Type: text/plain; charset="US-ASCII"; name="patch.txt" Content-Disposition: attachment; filename="patch.txt" Content-Transfer-Encoding: base64 Content-ID: X-Attachment-Id: f_kb8dnzc10 Y29tbWl0IDQ5NjUyYjdmNWI1N2I4OGMxZTBlMjdjZjhhYzQ4OGNiYzg1ZjFjN2QKQXV0aG9yOiBK b25hdGhhbiBXYWtlbHkgPGp3YWtlbHlAcmVkaGF0LmNvbT4KRGF0ZTogICBUdWUgSnVuIDkgMjE6 MjU6NTAgMjAyMCArMDEwMAoKICAgIGdjYy1jaGFuZ2Vsb2c6IEltcHJvdmUgZ2l0X2NvbW1pdC5w eSBkaWFnbm9zdGljcwogICAgCiAgICBUaGlzIGNoYW5nZXMgc29tZSBlcnJvciBtZXNzYWdlcyB0 byBiZSBtb3JlIHNlbGYtY29uc2lzdGVudCBhbmQgdG8gZml4CiAgICBzb21lIGdyYW1tYXIuCiAg ICAKICAgIGNvbnRyaWIvQ2hhbmdlTG9nOgogICAgCiAgICAgICAgICAgICogZ2NjLWNoYW5nZWxv Zy9naXRfY29tbWl0LnB5IChHaXRDb21taXQucGFyc2VfY2hhbmdlbG9nKToKICAgICAgICAgICAg SW1wcm92ZSBlcnJvciBzdHJpbmdzLgogICAgICAgICAgICAqIGdjYy1jaGFuZ2Vsb2cvdGVzdF9l bWFpbC5weTogVXBkYXRlIGV4cGVjdGVkIGVycm9ycy4KCmRpZmYgLS1naXQgYS9jb250cmliL2dj Yy1jaGFuZ2Vsb2cvZ2l0X2NvbW1pdC5weSBiL2NvbnRyaWIvZ2NjLWNoYW5nZWxvZy9naXRfY29t bWl0LnB5CmluZGV4IGY4NWQ0YzgzYzYzLi4wYjM1MGJhN2ZkYSAxMDA3NTUKLS0tIGEvY29udHJp Yi9nY2MtY2hhbmdlbG9nL2dpdF9jb21taXQucHkKKysrIGIvY29udHJpYi9nY2MtY2hhbmdlbG9n L2dpdF9jb21taXQucHkKQEAgLTM3Nyw4ICszNzcsOCBAQCBjbGFzcyBHaXRDb21taXQ6CiAgICAg ICAgICAgICAgICAgZWxpZiBhZGRpdGlvbmFsX2F1dGhvcl9yZWdleC5tYXRjaChsaW5lKToKICAg ICAgICAgICAgICAgICAgICAgbSA9IGFkZGl0aW9uYWxfYXV0aG9yX3JlZ2V4Lm1hdGNoKGxpbmUp CiAgICAgICAgICAgICAgICAgICAgIGlmIGxlbihtLmdyb3VwKCdzcGFjZXMnKSkgIT0gNDoKLSAg ICAgICAgICAgICAgICAgICAgICAgIG1zZyA9ICdhZGRpdGlvbmFsIGF1dGhvciBtdXN0IHByZXBl bmQgd2l0aCB0YWIgJyBcCi0gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAnYW5kIDQgc3Bh Y2VzJworICAgICAgICAgICAgICAgICAgICAgICAgbXNnID0gJ2FkZGl0aW9uYWwgYXV0aG9yIG11 c3QgYmUgaW5kZW50ZWQgd2l0aCAnXAorICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgJ29u ZSB0YWIgYW5kIGZvdXIgc3BhY2VzJwogICAgICAgICAgICAgICAgICAgICAgICAgc2VsZi5lcnJv cnMuYXBwZW5kKEVycm9yKG1zZywgbGluZSkpCiAgICAgICAgICAgICAgICAgICAgIGVsc2U6CiAg ICAgICAgICAgICAgICAgICAgICAgICBhdXRob3JfdHVwbGUgPSAobS5ncm91cCgnbmFtZScpLCBO b25lKQpAQCAtNDM4LDE1ICs0MzgsMTQgQEAgY2xhc3MgR2l0Q29tbWl0OgogICAgICAgICAgICAg ICAgICAgICBtID0gc3Rhcl9wcmVmaXhfcmVnZXgubWF0Y2gobGluZSkKICAgICAgICAgICAgICAg ICAgICAgaWYgbToKICAgICAgICAgICAgICAgICAgICAgICAgIGlmIGxlbihtLmdyb3VwKCdzcGFj ZXMnKSkgIT0gMToKLSAgICAgICAgICAgICAgICAgICAgICAgICAgICBlcnIgPSBFcnJvcignb25l IHNwYWNlIHNob3VsZCBmb2xsb3cgYXN0ZXJpc2snLAotICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgICAgIGxpbmUpCi0gICAgICAgICAgICAgICAgICAgICAgICAgICAgc2VsZi5l cnJvcnMuYXBwZW5kKGVycikKKyAgICAgICAgICAgICAgICAgICAgICAgICAgICBtc2cgPSAnb25l IHNwYWNlIHNob3VsZCBmb2xsb3cgYXN0ZXJpc2snCisgICAgICAgICAgICAgICAgICAgICAgICAg ICAgc2VsZi5lcnJvcnMuYXBwZW5kKEVycm9yKG1zZywgbGluZSkpCiAgICAgICAgICAgICAgICAg ICAgICAgICBlbHNlOgogICAgICAgICAgICAgICAgICAgICAgICAgICAgIGxhc3RfZW50cnkubGlu ZXMuYXBwZW5kKGxpbmUpCiAgICAgICAgICAgICAgICAgICAgIGVsc2U6CiAgICAgICAgICAgICAg ICAgICAgICAgICBpZiBsYXN0X2VudHJ5LmlzX2VtcHR5OgogICAgICAgICAgICAgICAgICAgICAg ICAgICAgIG1zZyA9ICdmaXJzdCBsaW5lIHNob3VsZCBzdGFydCB3aXRoIGEgdGFiLCAnIFwKLSAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAnYXN0ZXJpc2sgYW5kIHNwYWNlJworICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICdhbiBhc3RlcmlzayBhbmQgYSBzcGFjZScK ICAgICAgICAgICAgICAgICAgICAgICAgICAgICBzZWxmLmVycm9ycy5hcHBlbmQoRXJyb3IobXNn LCBsaW5lKSkKICAgICAgICAgICAgICAgICAgICAgICAgIGVsc2U6CiAgICAgICAgICAgICAgICAg ICAgICAgICAgICAgbGFzdF9lbnRyeS5saW5lcy5hcHBlbmQobGluZSkKQEAgLTUyNyw3ICs1MjYs NyBAQCBjbGFzcyBHaXRDb21taXQ6CiAgICAgICAgIHVzZWRfcGF0dGVybnMgPSBzZXQoKQogICAg ICAgICBmb3IgZW50cnkgaW4gc2VsZi5jaGFuZ2Vsb2dfZW50cmllczoKICAgICAgICAgICAgIGlm IG5vdCBlbnRyeS5maWxlczoKLSAgICAgICAgICAgICAgICBtc2cgPSAnQ2hhbmdlTG9nIG11c3Qg Y29udGFpbiBhdCBsZWFzdCBvbmUgZmlsZSBlbnRyeScKKyAgICAgICAgICAgICAgICBtc2cgPSAn bm8gZmlsZXMgbWVudGlvbmVkIGZvciBDaGFuZ2VMb2cgaW4gZGlyZWN0b3J5JwogICAgICAgICAg ICAgICAgIHNlbGYuZXJyb3JzLmFwcGVuZChFcnJvcihtc2csIGVudHJ5LmZvbGRlcikpCiAgICAg ICAgICAgICBhc3NlcnQgbm90IGVudHJ5LmZvbGRlci5lbmRzd2l0aCgnLycpCiAgICAgICAgICAg ICBmb3IgZmlsZSBpbiBlbnRyeS5maWxlczoKQEAgLTU0MCw3ICs1MzksOCBAQCBjbGFzcyBHaXRD b21taXQ6CiAgICAgICAgICAgICAgICAgaWYgbm90IHNlbGYuaXNfY2hhbmdlbG9nX2ZpbGVuYW1l KHhbMF0pXQogICAgICAgICBjaGFuZ2VkX2ZpbGVzID0gc2V0KGNhbmQpCiAgICAgICAgIGZvciBm aWxlIGluIHNvcnRlZChtZW50aW9uZWRfZmlsZXMgLSBjaGFuZ2VkX2ZpbGVzKToKLSAgICAgICAg ICAgIHNlbGYuZXJyb3JzLmFwcGVuZChFcnJvcignZmlsZSBub3QgY2hhbmdlZCBpbiBhIHBhdGNo JywgZmlsZSkpCisgICAgICAgICAgICBtc2cgPSAndW5jaGFuZ2VkIGZpbGUgbWVudGlvbmVkIGlu IGEgQ2hhbmdlTG9nJworICAgICAgICAgICAgc2VsZi5lcnJvcnMuYXBwZW5kKEVycm9yKG1zZywg ZmlsZSkpCiAgICAgICAgIGZvciBmaWxlIGluIHNvcnRlZChjaGFuZ2VkX2ZpbGVzIC0gbWVudGlv bmVkX2ZpbGVzKToKICAgICAgICAgICAgIGlmIG5vdCBzZWxmLmluX2lnbm9yZWRfbG9jYXRpb24o ZmlsZSk6CiAgICAgICAgICAgICAgICAgaWYgZmlsZSBpbiBzZWxmLm5ld19maWxlczoKZGlmZiAt LWdpdCBhL2NvbnRyaWIvZ2NjLWNoYW5nZWxvZy90ZXN0X2VtYWlsLnB5IGIvY29udHJpYi9nY2Mt Y2hhbmdlbG9nL3Rlc3RfZW1haWwucHkKaW5kZXggMDRkZGFkM2YxMDAuLmRmNTdiYjVjOTRhIDEw MDc1NQotLS0gYS9jb250cmliL2djYy1jaGFuZ2Vsb2cvdGVzdF9lbWFpbC5weQorKysgYi9jb250 cmliL2djYy1jaGFuZ2Vsb2cvdGVzdF9lbWFpbC5weQpAQCAtMTA1LDcgKzEwNSw3IEBAIGNsYXNz IFRlc3RHY2NDaGFuZ2Vsb2codW5pdHRlc3QuVGVzdENhc2UpOgogICAgICAgICBlbWFpbCA9IHNl bGYuZnJvbV9wYXRjaF9nbG9iKCcwMDk2JykKICAgICAgICAgYXNzZXJ0IGVtYWlsLmVycm9ycwog ICAgICAgICBlcnIgPSBlbWFpbC5lcnJvcnNbMF0KLSAgICAgICAgYXNzZXJ0IGVyci5tZXNzYWdl ID09ICdmaWxlIG5vdCBjaGFuZ2VkIGluIGEgcGF0Y2gnCisgICAgICAgIGFzc2VydCBlcnIubWVz c2FnZSA9PSAndW5jaGFuZ2VkIGZpbGUgbWVudGlvbmVkIGluIGEgQ2hhbmdlTG9nJwogICAgICAg ICBhc3NlcnQgZXJyLmxpbmUgPT0gJ2djYy90ZXN0c3VpdGUvZ2NjLnRhcmdldC9hYXJjaDY0Lycg XAogICAgICAgICAgICAgICAgICAgICAgICAgICAgJ2FkdnNpbWQtaW50cmluc2ljcy92ZG90LWNv bXBpbGUtMy0xLmMnCiAKQEAgLTE2MSw4ICsxNjEsOCBAQCBjbGFzcyBUZXN0R2NjQ2hhbmdlbG9n KHVuaXR0ZXN0LlRlc3RDYXNlKToKIAogICAgIGRlZiB0ZXN0X2FkZGl0aW9uYWxfYXV0aG9yX2xp c3Qoc2VsZik6CiAgICAgICAgIGVtYWlsID0gc2VsZi5mcm9tX3BhdGNoX2dsb2IoJzAzNDInKQot ICAgICAgICBhc3NlcnQgKGVtYWlsLmVycm9yc1sxXS5tZXNzYWdlID09ICdhZGRpdGlvbmFsIGF1 dGhvciBtdXN0IHByZXBlbmQgJwotICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgICd3aXRoIHRhYiBhbmQgNCBzcGFjZXMnKQorICAgICAgICBhc3NlcnQgKGVtYWlsLmVy cm9yc1sxXS5tZXNzYWdlID09ICdhZGRpdGlvbmFsIGF1dGhvciBtdXN0IGJlIGluZGVudGVkICcK KyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAnd2l0aCBvbmUgdGFi IGFuZCBmb3VyIHNwYWNlcycpCiAKICAgICBkZWYgdGVzdF90cmFpbGluZ193aGl0ZXNwYWNlcyhz ZWxmKToKICAgICAgICAgZW1haWwgPSBzZWxmLmdldF9naXRfZW1haWwoJ3RyYWlsaW5nLXdoaXRl c3BhY2VzLnBhdGNoJykKQEAgLTI2MCw4ICsyNjAsOCBAQCBjbGFzcyBUZXN0R2NjQ2hhbmdlbG9n KHVuaXR0ZXN0LlRlc3RDYXNlKToKIAogICAgIGRlZiB0ZXN0X3dyb25nX2NoYW5nZWxvZ19lbnRy eShzZWxmKToKICAgICAgICAgZW1haWwgPSBzZWxmLmZyb21fcGF0Y2hfZ2xvYignMDAyMC1JUEEt QXZvaWQnKQotICAgICAgICBhc3NlcnQgKGVtYWlsLmVycm9yc1swXS5tZXNzYWdlCi0gICAgICAg ICAgICAgICAgPT0gJ2ZpcnN0IGxpbmUgc2hvdWxkIHN0YXJ0IHdpdGggYSB0YWIsIGFzdGVyaXNr IGFuZCBzcGFjZScpCisgICAgICAgIG1zZyA9ICdmaXJzdCBsaW5lIHNob3VsZCBzdGFydCB3aXRo IGEgdGFiLCBhbiBhc3RlcmlzayBhbmQgYSBzcGFjZScKKyAgICAgICAgYXNzZXJ0IChlbWFpbC5l cnJvcnNbMF0ubWVzc2FnZSA9PSBtc2cpCiAKICAgICBkZWYgdGVzdF9jaGVycnlfcGlja19mb3Jt YXQoc2VsZik6CiAgICAgICAgIGVtYWlsID0gc2VsZi5mcm9tX3BhdGNoX2dsb2IoJzAwMDEtYy1B bGlhcy5wYXRjaCcpCg== --000000000000a2d7cd05a7ac96a6--