Re: [PATCH 2/2] Use $(git rev-parse --show-toplevel) in cd_to_toplevel()

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 11, 2010, 07:46 UTC
Message-ID: <7viqb9w0c8.fsf@alter.siamese.dyndns.org>
In-Reply-To: <alpine.LNX.2.00.1001112022190.8842@vqena.qenxr.bet.am>

Steven Drake <sdrake@xnet.co.nz> writes:

Show 10 quoted lines

>  as it gives the absolute (aka "physical") path of the toplevel directory
>  and 'cd -P' is not supported by all shell implementations.
>
> See NetBSD PR/42168.
> http://www.netbsd.org/cgi-bin/query-pr-single.pl?number=42168
> ---
> This is a revision of my original patch after the input from Jonathan.
> ---
>  git-sh-setup.sh |   18 ++++--------------
>  1 files changed, 4 insertions(+), 14 deletions(-)

The patch text may make sense (I haven't thought things through yet) but please make sure your proposed commit log messages conform to the local convention.

 (1) We don't chop a sentence in the middle of the subject line and
     continue the same sentence to the first line of the body of the
     message;

 (2) We don't indent the log message by one SP at the beginning of lines;

 (3) Please avoid referring to external resource in the commit log message
     whenever makes sense; the log should be understandable on its own.
     Because the first paragraph of your message describes the issue the
     patch addresses very well already, you don't need "See NetBSD..." and
     URL.  If you want to have them to help the reviewers, place such
     reference after the three-dash line, just like you wrote "This is a
     revision..."  You would help reviewers even more if you added a
     pointer to your earlier patch after that sentence;

 (4) Sign your patch, before the three-dash line.

Show 9 quoted lines

> diff --git a/git-sh-setup.sh b/git-sh-setup.sh
> index dfcb807..4ecc578 100755
> --- a/git-sh-setup.sh
> +++ b/git-sh-setup.sh
> @@ -120,20 +120,10 @@ is_bare_repository () {
>  }
>  
>  cd_to_toplevel () {
> +	cdup=$(git rev-parse --show-toplevel) && cd "$cdup" || {

Please line-break immediately after &&; it makes it easier to read in general, and it would make "cd" stand out in this particular case, as it is the most important part of this particular function.

Thanks.