patch submission guide (was Re: [PATCH] create and set temporary home directory)

[Florian Friesdorf]

Hi Carl,

On Fri, 06 May 2011 14:06:22 -0700, Carl Worth <cworth at cworth.org> wrote:
> On Mon, 18 Apr 2011 19:41:39 +0200, Florian Friesdorf <flo at chaoflow.net> 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 <COMMIT>^
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 <flo at chaoflow.net>
  GPG FPR: 7A13 5EEE 1421 9FC2 108D  BAAF 38F8 99A3 0C45 F083
Jabber/XMPP: flo at chaoflow.net
IRC: chaoflow on freenode,ircnet,blafasel,OFTC
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: <http://notmuchmail.org/pipermail/notmuch/attachments/20110507/5444a9ac/attachment.pgp>