From: David Bremner <david@tethera.net>
To: Erik Rybakken <erik.rybakken@math.ntnu.no>,
Jani Nikula <jani@nikula.org>,
notmuch@notmuchmail.org
Subject: Re: [PATCH] Add option `hooks.path` for setting the directory of hooks.
Date: Fri, 26 Aug 2016 07:57:06 -0300 [thread overview]
Message-ID: <87shtrol19.fsf@zancas.localnet> (raw)
In-Reply-To: <20160824215430.ljugc3vevx4ve2gq@dell>
Erik Rybakken <erik.rybakken@math.ntnu.no> writes:
> Hi again,
>
> I implemented the option for hooks myself. The patch is included. Please
> bear with me, this is my first contribution to notmuch (and my first
> attempt to write C code). I tested the option, and it seems to work.
>
> Best,
> Erik
Welcome. You may find https://notmuchmail.org/contributing/ has some
helpful hints, if you didn't already see it. In particular, your patch
needs an "meaningful commit message". You seem to have mostly managed to
follow the notmuch coding style, so thanks for that.
> +General
> +-------
> +
> +Add option `hooks.path` for setting the directory for hooks. If
> +unset, it will default to the `.notmuch/hooks` sub-directory.
> +
As a minor point, this should go under "Command Line Interface", since
hooks are strictly a CLI feature.
> +static const char hooks_config_comment[] =
> + " Hook configuration\n"
> + "\n"
> + " The only value supported here is 'path' which should be the directory\n"
> + " where your hooks exists.\n";
> +
"hooks exist."
No real objections to the code or the feature, but you'll need to update
the test suite. In particular T030-config and T040-setup are currently
broken by your patch (but easy to fix). You should also add at least
test to T400-hooks.sh to ensure running hooks from a non-standard
location works. Feel free to ask for help with the test suite, if
copying existing tests isn't enough.
d
next prev parent reply other threads:[~2016-08-26 10:57 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-24 16:30 Feature request: Finer control of paths Erik Rybakken
2016-08-24 17:45 ` Jani Nikula
2016-08-24 21:55 ` [PATCH] Add option `hooks.path` for setting the directory of hooks Erik Rybakken
2016-08-26 10:57 ` David Bremner [this message]
2016-08-26 11:32 ` Tomi Ollila
2016-08-27 13:28 ` Erik Rybakken
2016-08-30 5:43 ` Tomi Ollila
2016-08-30 8:49 ` Erik Rybakken
2016-08-30 11:43 ` David Bremner
2016-08-30 13:35 ` Tomi Ollila
2016-08-30 17:51 ` Tomi Ollila
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://notmuchmail.org/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87shtrol19.fsf@zancas.localnet \
--to=david@tethera.net \
--cc=erik.rybakken@math.ntnu.no \
--cc=jani@nikula.org \
--cc=notmuch@notmuchmail.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).