Hi Michael,

thanks for the patch,

> Thanks for the intial reivew. I updated the patch with a test and
> documentation for the new option. Happy to merge the test into one of
> the existing test files, I read t/README and greping around I did not
> find a place that looked like a good fit.

I think keeping tests as separate as possible is a good idea.
Looking at the patch https://public-inbox.org/git/20180413094314.GA2404@bod/

The patch seems reasonable, apart from minor nits:
In the test we'd prefer no whitespace on the right side of the redirection,
i.e. echo content >foo

Instead of evaluating git commands in shell and assigning it to a variable,
we'd prefer to dump it to files:

  git show HEAD:symlink >actual &&
  echo foo >expect &&
  test_cmp expect actual

(instead of content=$(git show HEAD:foo) && test $content == ...)

The reason for this is that the &&-chain will inspect the return code
of the git command.

There is a typo &e&.

Can we reword the documentation, such that we do not have
an occurrence of "extended SHA-1" ?
(By now the Git community came up with a plan to migrate
away from SHA-1, hence we'd not want to introduce more
dependencies even in the form of documentation for that)

Maybe

Follow symlinks inside the repository when requesting
objects in extended revision syntax of the form tree-ish:path-in-tree.

Thanks,
Stefan

Reply via email to