Re: [PATCH] create and set temporary home directory

Subject: Re: [PATCH] create and set temporary home directory

Date: Fri, 06 May 2011 14:06:22 -0700

To: Florian Friesdorf, notmuch@notmuchmail.org

Cc:

From: Carl Worth


On Mon, 18 Apr 2011 19:41:39 +0200, Florian Friesdorf <flo@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.

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

So with all of the above points taken together, your email might have
read something like what I've provided below.

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

Fortunately, I was able to recover from most of that with my dirvish
backups. But then I was also on vacation for several days.

Anyway, I'm excited to see the release candidate branch that jrollins is
putting together now. Hopefully I'll find some good way to collaborate
with the work he is doing.

-Carl

From: Florian Friesdorf <flo@chaoflow.net>, notmuch@notmuchmail.org
Subject: [PATCH] Create and set temporary home directory
To: notmuch@notmuchmail.org

Create and set temporary home directory
    
Within test/emacs two tests access the build users home
directory. These tests fail in case of a non-existent home directory.
    
Fix these tests to not touch the build user's home directory. Instead,
create a directory in the temporary test directory and set HOME
accordingly.

---

This is my first patch sent to the list. I'm not sure whether I've done
it properly.

 test/test-lib.sh |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
...
part-000.sig (application/pgp-signature)

Thread: