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, 09:58 UTC
Message-ID: <7vhbqtt13e.fsf@alter.siamese.dyndns.org>
In-Reply-To: <alpine.LNX.2.00.1001112114140.9352@vqena.qenxr.bet.am>

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

Show 7 quoted lines

>>  (4) Sign your patch, before the three-dash line.
>
> Opps forgot '--signoff', I've put format.signoff=ture in .gitconfig to solve
> that problem. 
>
> Perhaps a warning message from format-patch of the form:
> WARNING: You have not added a "Signed-off-by:" line did you mean to!

1. It is usually a good idea to make it a habit of running "commit -s",
   iow, record your sign-off at _commit time_, when working on a project
   that uses the convention.  You may start contributing by sending a
   pull-request instead of patches later.

2. scripts/checkpatch.pl script in the Linux kernel project is a good tool
   to check your patch before submission; you run it as:

   $ perl checkpatch.pl --no-tree 0001-my-changes.patch

Show 6 quoted lines

>> 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.
>
> Good point, althought did you mean as a general shell script coding rule or
> just in this particular case.

"In general" as a general style suggestion, and "show 'cd' at the beginning on its own line" as a more reason to do so for this specific case.

> Thanks for the feedback.  Did you want me to resend the signed and cleaned up
> patches direct to you?

Or to the list.

I could actually have fixed them up and commit myself, instead of responding with comments that can be seen as if I were nitpicking.

The reason why I chose not to was because I hoped you would keep contributing to this project more in the future (I see you have already another commit in our history, and you seem to be reasonably competent, judging from the patch description and the way you communicate in the discussion). And I wanted to make sure your future patches are easier to handle for me ;-).

Thanks.