Re: [PATCH v3 00/20] insert command

From: Jani Nikula <jani@nikula.org>
To: Peter Wang <novalazy@gmail.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH v3 00/20] insert command
Date: Tue, 22 Jan 2013 22:05:15 +0200	[thread overview]
Message-ID: <87zk01htl0.fsf@nikula.org> (raw)
In-Reply-To: <1358643004-14522-1-git-send-email-novalazy@gmail.com>

On Sun, 20 Jan 2013, Peter Wang <novalazy@gmail.com> wrote:
> This mainly addresses review comments from v2
> and rebases on top of tag-util.c.

Hi Peter, I have some interest in seeing this get merged. To get that
done in a timely manner, my suggestion is to do this in two steps. Drop
the --folder and --create-folder options and the related code for now,
and submit them as another series once the first part has been
merged. It will be faster to iterate the first part, and we can dodge
some of the issues in the --folder and --create-folder options for now.

This is just my personal opinion, and it's up to you whether you do this
or not. Let's just call it a hunch saying it'll be faster this way. ;)

The patches would be rearranged as follows:

> Peter Wang (20):
>   tag-util: do not reset list in parse_tag_command_line
>   tag-util: move out 'tag' command-line checks

Keep the above separate, and make them the first preparatory patches.

>   cli: add stub for insert command
>   insert: open Maildir tmp file
>   insert: copy stdin to Maildir tmp file
>   insert: move file from Maildir tmp to new
>   insert: add new message to database
>   insert: apply default tags to new message
>   insert: parse and apply command-line tag operations
>   insert: fsync after writing tmp file
>   insert: trap SIGINT and clean up
>   insert: add copyright line from notmuch-deliver

Squash all of the above together into one patch. I will send a review of
these based on this version of the series.

>   insert: support --folder option
>   insert: prevent writes outside Maildir hierarchy
>   insert: add --create-folder option
>   insert: fsync new directory after rename
>   insert: fsync parent directory after mkdir

Drop these for now.

>   man: document 'insert' command
>   man: reference notmuch-insert.1
>   test: add tests for insert

Keep these separate as they are, but update to reflect the above.

Thoughts?

BR,
Jani.

>
>  Makefile.local                  |   1 +
>  man/Makefile.local              |   1 +
>  man/man1/notmuch-config.1       |   4 +-
>  man/man1/notmuch-count.1        |   4 +-
>  man/man1/notmuch-dump.1         |   4 +-
>  man/man1/notmuch-insert.1       |  59 +++++
>  man/man1/notmuch-new.1          |   4 +-
>  man/man1/notmuch-reply.1        |   3 +-
>  man/man1/notmuch-restore.1      |   3 +-
>  man/man1/notmuch-search.1       |   3 +-
>  man/man1/notmuch-show.1         |   3 +-
>  man/man1/notmuch-tag.1          |   3 +-
>  man/man1/notmuch.1              |   3 +-
>  man/man5/notmuch-hooks.5        |   4 +-
>  man/man7/notmuch-search-terms.7 |   3 +-
>  notmuch-client.h                |   3 +
>  notmuch-insert.c                | 482 ++++++++++++++++++++++++++++++++++++++++
>  notmuch-tag.c                   |  10 +
>  notmuch.c                       |   3 +
>  tag-util.c                      |  12 -
>  tag-util.h                      |   2 +
>  test/insert                     | 106 +++++++++
>  test/notmuch-test               |   1 +
>  23 files changed, 692 insertions(+), 29 deletions(-)
>  create mode 100644 man/man1/notmuch-insert.1
>  create mode 100644 notmuch-insert.c
>  create mode 100755 test/insert
>
> -- 
> 1.7.12.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch