On Mon, Jun 21, 2021 at 8:34 AM Tom Lane <t...@sss.pgh.pa.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
git-blame-ignore-revs
Description: Binary data
