.git-blame-ignore-revs lists commit hashes that should be skipped by git blame e.g. non-functional whitespace or style cleanup.
Details
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
I added two commits as an example; if we decide to go ahead I will do a first pass to collect suitable commits
I'm cautiously almost not pessimistic about this...
It will cut down the noise a bit... but the cost of maintaining this list seems high... and it's unclear the performance impact if we have thousands of items on this list, as seems quite likely...
So if we have at least a semi-sane story for both of these answers that means I'll almost never have to worry about it, it would, to quite Bruce, depessmize my outlook :)
it's unclear the performance impact if we have thousands of items on this list, as seems quite likely...
FWIW it needs to be explicitly specified to git blame or configured
GitHub recently started to parse this by default so I'd guess they are confident it's no a significant performance impact. Anywhere you'd have thousands of entries you're also parsing a significant portion of the pack files so parsing one text file into some sort of hash structure seems unlikely to be a big deal.
I'm not worried about things being missing. Developer should and entries that annoy them, but there's not need to be perfect. I'm a little worried there might be some arguments about edge cases, but I think we can codify better policy if and when it becomes an issue.
.git-blame-ignore-revs | ||
---|---|---|
4 | Maybe include the git config command to enable this file for the repo? |
How are you going to generate this file?
I imagine starting with something like this to find the largest whitespace-only changes:
git rev-list --no-merges HEAD | while read hash; do if [ -z "$(git diff -w $hash^..$hash)" ]; then echo -n "$hash " git diff $hash^..$hash | wc -l fi done | sort -rnk2
It will (slowly) produce something like:
2c9764f36b6f20e9a6c71ce64a21988a394050b6 34916
701301511f0ca044fc71470ed7842cd2c6fee001 12741
4cdc5e12a849871e4e8062a62a31f28545901d84 7424
44bc30192139b0b3c95510ab3b35802bcc6d63e4 5842
29e54af43ee06ed5636b9f567795cd271efc1ef7 1625
a0358e3d5184950b4316f105eb292cbafdea208b 1454
c2fa905cf6c1cf1938a0353679e3bd0b617ca179 1184
86d69de88d1c7e8c7ed906590652008fc6f44d05 705
efbe2af2ce5e444aa06d6d989c147ecea3f3120f 565
21ab8c75c940dd15343b4af28b18a66f377e670a 415
or instead of rev-list --no-merges can use git log --format=%H --grep 'style(9)', but will have to inspect each change to ensure that it is style-only.
But these are just approaches to find candidates for this file. I suspect generally entries will be added by developers landing on a whitespace commit during git blame
Ok it looks like the top twenty whitespace-only commits are:
# regen syscall files after d51198d63b63 2c9764f36b6f20e9a6c71ce64a21988a394050b6 # unexpand -a everything 014585d12527a3c053778459bbd93cdb37421430 # Remove whitespace at EOL. 7ebcc426efefea9cd16ac6f565e72030c608f7a4 # ipfilter module: Style(9) requires a space after return 701301511f0ca044fc71470ed7842cd2c6fee001 # Removed whitespace at end-of-line; no content changes. I simply did cd src/share; find man[1-9] -type f|xargs perl -pi -e 's/[ \t]+$//' c1f3e4bf21571a88d4269a87f57e3e4ff2ba3ef6 # Remove whitespace at EOL. f247324df75b7f55b48b92acb3b42a5ae2deac8a # ixgbe: remove whitespace in function comments 4cdc5e12a849871e4e8062a62a31f28545901d84 # s/filesystem/file system/g as discussed on -developers ce66ddb76352a2e5f34aacdbe7733d92e60aff17 # Remove trailing whitespace per mdoc lint warning 50d675f7a9fca2f3fba3a23ac41a8ae2c0162e4e # ipfilter userland: Style(9) requires a space after return 44bc30192139b0b3c95510ab3b35802bcc6d63e4 # First pass of style(9) for #define's. ddd36f9facc3a776056bb354bb9b13dcf3d9ccf5 # White space cleanup for netinet before branch: a4f757cd5dc1421d90d72a34853247e0c2c18cce # Fix style nit noticed by bde. 78fd9f198c7e78f4fe384ec772cc908631bec6ee # `unexpand -a' should be run _before_ sed 's/^#define /#define^I/g'. d93389d676c34ab28aa75b3bbc4cd9a8b5dc86e5 # style(9): clean up trailing whitespace e1d581b289848ffa97c452756dc52d45efb68a01 # Remove all trailing white space from the BBR/Rack fold. Bits left around by emacs (thanks emacs). 3fba40d9f2b3551ec627891640dd63829dc13fba # In kernel config files, it is supposed to be 'options<space><tab>' not 'options<tab><tab>', per long standing (but recently not so strictly enforced) convention. d4f95c889dd2cc01eca256766fe5ccb289e0750a # Remove trailing whitespace 27cc91fbf84cc6c1cef81df859125f42292a2b69 # - Use "device\t" and "options \t" for consistency. b3b17597eab61d4ea34f1cfd653c53d284e18c75 # libc: use standard LF line endings, not CRLF 29e54af43ee06ed5636b9f567795cd271efc1ef7
Followup question: Should we have the people making typos comment changes add their commits here?
The larger question is 'when should we add things here'? Is the cost such that we want to do the 'top commits' only, or 'every boring commit' or somewhere in between (and where on that scale should we advise people).
I think I lean toward "add them when their absence is annoying" as an initial policy. It might be worth some experiments to see if the result of adding all whitespace-only commits is measurable though.