On 6/2/20 4:14 PM, Jonathan Wakely wrote:
On Tue, 2 Jun 2020 at 14:56, Jonathan Wakely <jwakely....@gmail.com> wrote:

On Tue, 2 Jun 2020 at 14:16, Martin Liška <mli...@suse.cz> wrote:

On 6/2/20 1:48 PM, Martin Liška 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 = '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.

Martin


Do you like the direction?


Reply via email to