8b/cfc9ab414d52891f36055eac5094a192df2a72

   1 Return-Path: <david@tethera.net>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 1C5C9431FC4\r
   6         for <notmuch@notmuchmail.org>; Mon,  3 Nov 2014 23:05:29 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: 0\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none]\r
  12         autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id snuFFBZx9aLm for <notmuch@notmuchmail.org>;\r
  16         Mon,  3 Nov 2014 23:05:22 -0800 (PST)\r
  17 Received: from yantan.tethera.net (yantan.tethera.net [199.188.72.155])\r
  18         (using TLSv1 with cipher DHE-RSA-AES128-SHA (128/128 bits))\r
  19         (No client certificate requested)\r
  20         by olra.theworths.org (Postfix) with ESMTPS id 1F74F431FAE\r
  21         for <notmuch@notmuchmail.org>; Mon,  3 Nov 2014 23:05:22 -0800 (PST)\r
  22 Received: from remotemail by yantan.tethera.net with local (Exim 4.80)\r
  23         (envelope-from <david@tethera.net>)\r
  24         id 1XlYB7-0001pe-DF; Tue, 04 Nov 2014 03:05:21 -0400\r
  25 Received: (nullmailer pid 26803 invoked by uid 1000); Tue, 04 Nov 2014\r
  26         07:05:16 -0000\r
  27 From: David Bremner <david@tethera.net>\r
  28 To: Michal Sojka <sojkam1@fel.cvut.cz>, notmuch@notmuchmail.org\r
  29 Subject: Re: [PATCH v2 08/10] cli: address: Do not output duplicate addresses\r
  30 In-Reply-To: <1415058622-21162-9-git-send-email-sojkam1@fel.cvut.cz>\r
  31 References: <1415058622-21162-1-git-send-email-sojkam1@fel.cvut.cz>\r
  32         <1415058622-21162-9-git-send-email-sojkam1@fel.cvut.cz>\r
  33 User-Agent: Notmuch/0.18.2+156~g3cc8ed5 (http://notmuchmail.org) Emacs/24.4.1\r
  34         (x86_64-pc-linux-gnu)\r
  35 Date: Tue, 04 Nov 2014 08:05:16 +0100\r
  36 Message-ID: <87a947monn.fsf@maritornes.cs.unb.ca>\r
  37 MIME-Version: 1.0\r
  38 Content-Type: text/plain\r
  39 X-BeenThere: notmuch@notmuchmail.org\r
  40 X-Mailman-Version: 2.1.13\r
  41 Precedence: list\r
  42 List-Id: "Use and development of the notmuch mail system."\r
  43         <notmuch.notmuchmail.org>\r
  44 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  45         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  46 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  47 List-Post: <mailto:notmuch@notmuchmail.org>\r
  48 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  49 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  50         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  51 X-List-Received-Date: Tue, 04 Nov 2014 07:05:29 -0000\r
  52 \r
  53 Michal Sojka <sojkam1@fel.cvut.cz> writes:\r
  54 \r
  55 >  \r
  56 > +/* Returns TRUE iff name and addr is duplicate. */\r
  57 \r
  58 If you're revising this patch, it would be good to mention the side\r
  59 effect of this function.\r
  60 \r
  61 > -process_address_list (const search_context_t *ctx, InternetAddressList *list)\r
  62 > +process_address_list (const search_context_t *ctx,\r
  63 > +                   InternetAddressList *list)\r
  64 \r
  65 It probably doesn't make any difference, but this looks like a needless\r
  66 whitespace change.\r
  67 \r
  68 This function definitely needs some comment / pointer to\r
  69 documention. And probably not to have _my in the name.\r
  70 \r
  71 > +static void\r
  72 > +_my_talloc_free_for_g_hash (void *ptr)\r
  73 > +{\r
  74 > +    talloc_free (ptr);\r
  75 > +}\r
  76 > +\r
  77 \r
  78 I don't understand the name of the next subtest\r
  79 \r
  80 > +test_begin_subtest "No --output"\r
  81 > +notmuch address --output=sender --output=recipients '*' >OUTPUT\r
  82 > +# Use EXPECTED from previous subtest\r
  83 > +test_expect_equal_file OUTPUT EXPECTED\r
  84 > +\r
  85 > +\r
  86 > +test_done\r
  87 \r
  88 nitpick, extra blank lines\r
  89 \r
  90 So, AIUI, this is all of the series proposed for 0.19. It looks close to\r
  91 OK to me, modulo some minor style nits. One anonymous commentator on\r
  92 IRC mentioned the use of module scope variables, I guess in patch\r
  93 6/10. I'm not sure of a better solution, but it's true in a perfect\r
  94 world we wouldn't have module local state.\r
  95 \r
  96 d\r