From: Tomi Ollila <tomi.ollila@iki.fi>
To: Mike Kelly <pioto@pioto.org>, notmuch@notmuchmail.org
Subject: Re: FreeBSD Support Patches
In-Reply-To: <1337953411-21000-1-git-send-email-pioto@pioto.org>
References: <1337953411-21000-1-git-send-email-pioto@pioto.org>
User-Agent: Notmuch/0.13+38~g944a859 (http://notmuchmail.org) Emacs/23.1.1
	(x86_64-redhat-linux-gnu)
Date: Fri, 25 May 2012 23:53:20 +0300
Message-ID: <m2k400geyn.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Fri, May 25 2012, Mike Kelly <pioto@pioto.org> wrote:

> Hi,
>
> These patches add some changes necessary for a clean build on FreeBSD,
> and for most of the tests to pass.
>
> Also mixed in are a few patches to disable emacs tests when you don't
> have emacs, along with tests that depend upon emacs. I could split those
> off onto a separate branch if required.

Yes...

Thank you for your efforts to make notmuch better; some general
comments on this patch series:


Commit messages are somewhat terse
check what http://notmuchmail.org/patchformatting/
has to say about this.

MANPATH setting has hit me too -- but that shold be set
in the same place where PATH is adjusted to find notmuch
command instead of hardcoding '../../man'.

s/executable/+111/ (and following sed (which could be combined
into next sed but that's another matter)) is good (that hit me
once too) -- This particular patch has the worst commit message,
though (Make test/basic more portable) ;/

in 6/10 there is missing space before sed in '...wc -l |sed...'.

-p overrides -n in perl, so perl -pe... (-npe confuses, at least
me...)

The patch series is supposed to add FreeBSD support; 
prereq additions to take care missing emacs should
be in separate series.

Some of the patches could be marked as 'trivial', putting those
to 'maybe ready' section in http://nmbug.tethera.net/status/
-- just if those had more comprehensive commit messages.

> You can find the latest version of these patches on my github repo's
> fbsd-support branch:
>
>   https://github.com/pioto/notmuch/compare/master...fbsd-support 

Tomi