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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, 10 Jan 2010, Junio C Hamano wrote:

>  (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;

Wondered wether I should have put the extra info after the three-dashes or
not, now I know.

I also made sure my second email had References and In-Reply-To headers to 
my first email.

>  (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!

> 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.

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

-- 
Steven
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]