Re: Maintaining a list of pgindent commits for "git blame" to ignore

From: Peter Geoghegan <pg(at)bowt(dot)ie>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Bruce Momjian <bruce(at)momjian(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Maintaining a list of pgindent commits for "git blame" to ignore
Date: 2021-06-21 23:47:09
Message-ID: CAH2-WzkNvyS7iYmUK59QexXkqnwPyX9SgrcfVxDT1hAC8pxL5Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jun 21, 2021 at 8:34 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> It's possible that some of these touch few enough lines that they
> are not worth listing; I did not check the commit delta sizes.

Commits that touch very few lines weren't included originally, just
because it didn't seem necessary. Even still, I've looked through the
extra commits now, and everything that you picked out looks in scope.
I'm just going to include these extra commits.

Attached is a new version of the same file, based on your feedback
(with those extra commits, and some commits from the last version
removed). I'll produce a conventional patch file in the next revision,
most likely.

> Meh. I can get on board with the idea of adding commit+revert pairs
> to this list, but I'd like a more principled selection filter than
> "which ones bother Peter". Maybe the size of the reverted patch
> should factor into it

I have to admit that you're right. That was why I picked those two
out. Of course I can defend this choice in detail, but in the interest
of not setting a terrible precedent I won't do that. The commits in
question have been removed from this revised version.

I think it's important that we not get into the business of adding
stuff to this willy-nilly. Inevitably everybody will have their own
pet peeve noisy commit, and will want to add to the list -- just like
I did. Naturally nobody will be interested in arguing against
including whatever individual pet peeve commit each time this comes
up. Regardless of the merits of the case. Let's just not go there
(unless perhaps it's truly awful for almost everybody).

> Do we have an idea of how much adding entries to this list slows
> down "git blame"? If we include commit+revert pairs more than
> very sparingly, I'm afraid we'll soon have an enormous list, and
> I wonder what that will cost us.

I doubt it costs us much, at least in any way that has a very
noticeable relationship as new commits are added. I've now settled on
68 commits, and expect that this won't need to grow very quickly, so
that seems fine. From my point of view it makes "git blame" far more
useful.

LLVM uses a file with fewer entries, and have had such a file since last year:

https://github.com/llvm/llvm-project/blob/main/.git-blame-ignore-revs

The list of commit hashes in the file that the Blender project uses is
about the same size:

https://github.com/blender/blender/blob/master/.git-blame-ignore-revs

We're using more commits than either project here, but it's within an
order of magnitude. Note that this is going to be opt-in, not opt-out.
It won't do anything unless an individual hacker decides to enable it
locally.

The convention seems to be that it is located in the top-level
directory. ISTM that we should follow that convention, since following
the convention is good, and does not in itself force anybody to ignore
any of the listed commits. Any thoughts on that?

--
Peter Geoghegan

Attachment Content-Type Size
git-blame-ignore-revs application/octet-stream 8.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2021-06-21 23:51:19 Re: disfavoring unparameterized nested loops
Previous Message Jim Nasby 2021-06-21 23:19:27 Assertion failure in HEAD and 13 after calling COMMIT in a stored proc