Hi Carl, On Fri, 06 May 2011 14:06:22 -0700, Carl Worth wrote: > On Mon, 18 Apr 2011 19:41:39 +0200, Florian Friesdorf wrote: > > My first patch send to the list, not sure whether done properly. > > Just fine, Florian. Thanks for the contribution! > > One small thing you might do differently in the future is to tweak the > email message to read exactly like a commit message. For example, a > sentence like the above "not sure whether done properly" is fine in an > email message, but doesn't make sense in the commit message. > > > I think the tests should not touch the build user's home directory. The > > patch creates a directory in the temporary test directory and sets home > > accordingly. > > Similarly, everything in a commit message is known to be your opinion, > so you should omit phrases like "I think". Instead, you should describe > what the commit actually does, and then describe why it does that. That text was intended as a comment (cover letter according to what I just learned) and the commit message itself was intended to be more to the point. > Finally, this little separator with three dashes: > > > --- > > test/test-lib.sh | 7 +++++++ > > 1 files changed, 7 insertions(+), 0 deletions(-) > > Is what lets git know where your commit message ends. Anything you > include after that (And before the patch itself) will be ignored by > git. So that's the perfect place to put a sentence like "My first > patch---not sure whether done properly". I now sent a patch with a cover letter using only git (search --format=sanitized_text): git format-patch --cover-letter ^ git send-email 000* If everything is correct, I would conclude this into a short howto on notmuchmail.org. So please, if there is still room for improvement, let me know. Do we prefer sendemail.chainreplyto or the new shallow format for patch series [1]? Should people who have a public git repository use it to publish their patches (in addition to sending them here / instead of)? I have seen (amdragon I think) the concept of for-review/... branches. Would that be a best practice? > I applied this patch last week, and would have pushed it, except that > just after applying it, I also tried cleaning up some of this part of > the code. And in the process it seems I managed to get the test suite to > run "rm -rf ${HOME}" with my actual home directory (oops!). I got a bit afraid, shouldnt my patch prevent that? I cannot imagine it caused it. [1] https://felipec.wordpress.com/2009/10/25/git-send-email-tricks/ regards florian -- Florian Friesdorf GPG FPR: 7A13 5EEE 1421 9FC2 108D BAAF 38F8 99A3 0C45 F083 Jabber/XMPP: flo@chaoflow.net IRC: chaoflow on freenode,ircnet,blafasel,OFTC