Re: [PATCH v2 13/27] ls-refs: introduce ls-refs server command

From: Stefan Beller <sbeller@google.com>
Date: Jan 26, 2018, 22:20 UTC
Message-ID: <CAGZ79kb14ciUoyHLAxmh0jGwwPYFVjGayrn9R2f+4+88A8=xsA@mail.gmail.com>
In-Reply-To: <20180125235838.138135-14-bmwill@google.com>

On Thu, Jan 25, 2018 at 3:58 PM, Brandon Williams <bmwill@google.com> wrote:

Show 7 quoted lines

> +ls-refs takes in the following parameters wrapped in packet-lines:
> +
> +    symrefs
> +       In addition to the object pointed by it, show the underlying ref
> +       pointed by it when showing a symbolic ref.
> +    peel
> +       Show peeled tags.

Would it make sense to default these two to on, and rather have optional no-symrefs and no-peel ?

That would save bandwidth in the default case, I would think.

Show 8 quoted lines

> +       cat >expect <<-EOF &&
> +       $(git rev-parse HEAD) HEAD
> +       $(git rev-parse refs/heads/dev) refs/heads/dev
> +       $(git rev-parse refs/heads/master) refs/heads/master
> +       $(git rev-parse refs/heads/release) refs/heads/release
> +       $(git rev-parse refs/tags/annotated-tag) refs/tags/annotated-tag
> +       $(git rev-parse refs/tags/one) refs/tags/one
> +       $(git rev-parse refs/tags/two) refs/tags/two

Invoking rev-parse quite a few times? I think the test suite is a trade off between readability ("what we expect the test to do and test") and speed (specifically on Windows forking is expensive);

I tried to come up with a more concise way to create this expectation using git-rev-parse, but did not find a good way to do so.

However maybe

  git for-each-ref --format='%(*objectname) %(*refname)' >expect

might help in reproducing the expected message? The downside of this would be to have to closely guard which refs are there though. I guess the '--pattern' could help there as it may be the same pattern as the input to the ls-refs. This might be too abstract for a test though.

I dunno.

Stefan