From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 20 Jul 2010 17:42:51 GMT
Subject: Re: [PATCH 2/4] git-shell-commands: Add a command to list bare repos
Message-ID: <7vlj96m4mc.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1279602972-1264-3-git-send-email-gdb@mit.edu>

Greg Brockman <gdb@MIT.EDU> writes:

> Signed-off-by: Greg Brockman <gdb@mit.edu>
> ---
>  contrib/git-shell-commands/list |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
>  create mode 100755 contrib/git-shell-commands/list
>
> diff --git a/contrib/git-shell-commands/list b/contrib/git-shell-commands/list
> new file mode 100755
> index 0000000..cd8b15a
> --- /dev/null
> +++ b/contrib/git-shell-commands/list
> @@ -0,0 +1,5 @@
> +#!/bin/sh
> +set -eu
> +
> +print_if_bare_repo='[ "$(git --git-dir="$1" rev-parse --is-bare-repository)" = true ] && echo "${1#./}"'

That's a very long line you have here.  It might be better to do split the
line perhaps like this for readability:

print_if_bare_repo='
	if "$(git --git-dir="$1" rev-parse --is-bare-repository)" = true
        then
	        printf "%s\n" "${1#./}"
	fi
'

It is unclear why it limits its listing only to bare repositories.  "It's
my design decision" is a perfectly acceptable answer, but no matter what
the reasoning is, it needs to be documented as a part of "How to use this"
insn to the users.  A separate file README in contrib/git-shell-commands
that reads like:

	Any bare repository whose name ends with ".git" under your home
	directory is visible by "list" extended command (no other git
	repositories are visible).

would probably be a good start.

> +find -type d -name "*.git" -exec sh -c "$print_if_bare_repo" -- \{} \; -prune

Also do you need "set -eu" at the beginning?  I don't see it serving a
useful purpose (other than being a development aid, that is).