On Wed, Apr 10 2019, Barret Rhoden wrote:
(Just skimming)
> revisions for commits that perform mass reformatting, and their users
> have the optional to ignore all of the commits in that file.
s/have the optional/have the option/
> +--ignore-revs-file <file>::
> + Ignore revisions listed in `file`, one unabbreviated object name per line.
> + Whitespace and comments beginning with `#` are ignored.
Maybe just say "Ignore revisions listed in `file`, which is expected to
be in the same format as an `fsck.skipList`.".
> + the `blame.ignoreRevsFile` config option. An empty file name, `""`, will
> + clear the list of revs from previously processed files.
Maybe I haven't read this carefully enough but the use-case for this
doesn't seem to be explained, you need this for the option, but the
config file too? If I want to override fsck.skipList I do
`fsck.skipList=/dev/zero`. Isn't that enough for this use-case without
introducing config state-machine magic?
> + split[0].unblamable = e->unblamable;
> + split[1].unblamable = e->unblamable;
> + split[2].unblamable = e->unblamable;
I wonder what the comfort level for people in general is before turning
this sort of thing into a for-loop, 4? :)
> + nr_lines = e->num_lines; // e changes in the loop
A C++-like trailing comment.
> + grep "^[0-9a-f]\+ [0-9]\+ 1" blame_raw | sed -e "s/ .*//" >actual &&
> + git rev-parse X >expect &&
> + test_cmp expect actual &&
> +
> + grep "^[0-9a-f]\+ [0-9]\+ 2" blame_raw | sed -e "s/ .*//" >actual &&
> + git rev-parse X >expect &&
> + test_cmp expect actual
The grep here is a bug. See my 4abf20f004 ("tests: fix unportable "\?"
and "\+" regex syntax", 2019-02-21).
