| Message ID | 20241016182124.48148-3-wolf@oriole.systems (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | builtin/shortlog: explicitly set hash algo when there is no repo | expand |
On Wed, Oct 16, 2024 at 08:21:24PM +0200, Wolfgang Müller wrote: > git-shortlog(1) might be used outside of a git repository to process a > log obtained from a repository initialized with --object-format=sha256. > Since git-shortlog(1) has no information on the hash algorithm, make > sure that it can still successfully process the log regardless. > > Signed-off-by: Wolfgang Müller <wolf@oriole.systems> > Thanks-to: Patrick Steinhardt <ps@pks.im> Same note here about ordering Signed-off-by: and Thanks-to: lines as in the previous patch. > @@ -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. Thanks, Taylor
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 :-)
On Wed, Oct 16, 2024 at 09:35:46PM +0200, Wolfgang Müller wrote: > > > @@ -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 :-) I have a vague preference towards just getting rid of it, but I wouldn't be sad to see it stay, either. Thanks, Taylor
diff --git a/t/t4201-shortlog.sh b/t/t4201-shortlog.sh index 0cc2aa5089..50d987cbe4 100755 --- a/t/t4201-shortlog.sh +++ b/t/t4201-shortlog.sh @@ -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 && + test_commit -C repo initial && + git -C repo log HEAD >log && + nongit git shortlog <log +' + test_expect_success 'shortlog should add newline when input line matches wraplen' ' cat >expect <<\EOF && A U Thor (2):
git-shortlog(1) might be used outside of a git repository to process a log obtained from a repository initialized with --object-format=sha256. Since git-shortlog(1) has no information on the hash algorithm, make sure that it can still successfully process the log regardless. Signed-off-by: Wolfgang Müller <wolf@oriole.systems> Thanks-to: Patrick Steinhardt <ps@pks.im> --- t/t4201-shortlog.sh | 8 ++++++++ 1 file changed, 8 insertions(+)