On 2024-10-16 15:25, Taylor Blau wrote: > Same note here about ordering Signed-off-by: and Thanks-to: lines as in > the previous patch. Shall keep in mind, thanks! > > @@ -147,6 +147,14 @@ fuzz() > > echo | nongit git shortlog --author=author > > ' > > > > +test_expect_success 'shortlog from non-git directory reads log from SHA256 repository' ' > > + test_when_finished "rm -rf repo" && > > + git init --object-format=sha256 repo && > > I wondered why we needed a separate repo for this test, but I understand > now that we need to have a SHA-256 repository to test this on. > > I'm still dubious as to the value of this test overall, but I think it > may be nice to s/repo/sha256-repo/ or something here to indicate its > purpose more clearly in the test itself. > > I don't feel strongly about it, though. I personally lean towards skipping out on this test, but I can also see value in it coming from a point of view of future changes to git-shortlog(1) behaviour. Then again, perhaps it would be better then to add a relevant test only if new behaviour is added. No strong feelings here either, I'd defer to what regular maintainers and developers here prefer :-) -- Wolf
