Re: [RFC PATCH] Add "git show --follow-symlinks HEAD:symlink"

From: Stefan Beller <sbeller@google.com>
Date: Apr 13, 2018, 17:28 UTC
Message-ID: <CAGZ79kZDUnxkKfqRnZiLhweJ4HNO5sSGUfxkEvJPGagV4wmtkg@mail.gmail.com>
In-Reply-To: <20180413094314.GA2404@bod>

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