Re: [PATCH 2/2] Use $(git rev-parse --show-toplevel) in cd_to_toplevel()
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:47:59
Steven Drake [off-list ref] writes:
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.
quoted hunk
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.