18/29d824000afd0d9fdde49a7bbd755c9fe4c408

   1 Return-Path: <jan@ryngle.com>\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 0731E431FBC\r
   6         for <notmuch@notmuchmail.org>; Wed, 25 Nov 2009 13:50:14 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 Received: from olra.theworths.org ([127.0.0.1])\r
   9         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  10         with ESMTP id MIhaGUo0jeoG for <notmuch@notmuchmail.org>;\r
  11         Wed, 25 Nov 2009 13:50:13 -0800 (PST)\r
  12 Received: from mail-ew0-f213.google.com (mail-ew0-f213.google.com\r
  13         [209.85.219.213])\r
  14         by olra.theworths.org (Postfix) with ESMTP id 39848431FAE\r
  15         for <notmuch@notmuchmail.org>; Wed, 25 Nov 2009 13:50:13 -0800 (PST)\r
  16 Received: by ewy5 with SMTP id 5so171946ewy.30\r
  17         for <notmuch@notmuchmail.org>; Wed, 25 Nov 2009 13:50:12 -0800 (PST)\r
  18 Received: by 10.213.24.22 with SMTP id t22mr137398ebb.59.1259185812010;\r
  19         Wed, 25 Nov 2009 13:50:12 -0800 (PST)\r
  20 Received: from x61s.janakj ([213.192.30.141])\r
  21         by mx.google.com with ESMTPS id 10sm260732eyz.7.2009.11.25.13.50.10\r
  22         (version=TLSv1/SSLv3 cipher=RC4-MD5);\r
  23         Wed, 25 Nov 2009 13:50:10 -0800 (PST)\r
  24 Received: by x61s.janakj (Postfix, from userid 1000)\r
  25         id E2A57440655; Wed, 25 Nov 2009 22:50:08 +0100 (CET)\r
  26 Date: Wed, 25 Nov 2009 22:50:08 +0100\r
  27 From: Jan Janak <jan@ryngle.com>\r
  28 To: Bart Trojanowski <bart@jukie.net>\r
  29 Message-ID: <20091125215008.GG25119@ryngle.com>\r
  30 References: <1259100630-13673-1-git-send-email-jan@ryngle.com>\r
  31         <1259100630-13673-2-git-send-email-jan@ryngle.com>\r
  32         <1259100630-13673-3-git-send-email-jan@ryngle.com>\r
  33         <20091125205639.GJ29312@jukie.net>\r
  34 MIME-Version: 1.0\r
  35 Content-Type: text/plain; charset=us-ascii\r
  36 Content-Disposition: inline\r
  37 In-Reply-To: <20091125205639.GJ29312@jukie.net>\r
  38 User-Agent: Mutt/1.5.18 (2008-05-17)\r
  39 Cc: notmuch@notmuchmail.org\r
  40 Subject: Re: [notmuch] [PATCH 2/4] notmuch: Config option to specify tags       to\r
  41         be applied by 'notmuch new'.\r
  42 X-BeenThere: notmuch@notmuchmail.org\r
  43 X-Mailman-Version: 2.1.12\r
  44 Precedence: list\r
  45 List-Id: "Use and development of the notmuch mail system."\r
  46         <notmuch.notmuchmail.org>\r
  47 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  48         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  49 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  50 List-Post: <mailto:notmuch@notmuchmail.org>\r
  51 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  52 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  53         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  54 X-List-Received-Date: Wed, 25 Nov 2009 21:50:14 -0000\r
  55 \r
  56 Hi Bart,\r
  57 \r
  58 On 25-11 15:56, Bart Trojanowski wrote:\r
  59 > Jan,\r
  60 > \r
  61 > I really want this feature to get in, so I am going to do my best to\r
  62 > review your code :)\r
  63 > \r
  64 > Here are some more sticking points...\r
  65 > \r
  66 > > +char **\r
  67 > > +notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length);\r
  68 > \r
  69 > If you are not giving over control of the pointer to the caller please\r
  70 > return const char * const *.\r
  71 \r
  72 I followed Carl's style there, in particular the following function:\r
  73 notmuch_config_get_user_other_email\r
  74 \r
  75 I can, of course, change that. But maybe we should wait for Carl to see\r
  76 which way he prefers.\r
  77 \r
  78 > Similarly...\r
  79 > \r
  80 > > +    char **new_tags;\r
  81 > \r
  82 > ... this should probably be const char **.\r
  83 \r
  84 That's the same story. I followed user_other_email there.\r
  85 \r
  86 > Next...\r
  87 > \r
  88 > > +char **\r
  89 > > +notmuch_config_get_new_tags (notmuch_config_t *config, size_t *length)\r
  90 > \r
  91 > ... but ...\r
  92 > \r
  93 > > +    unsigned int count, i;\r
  94 > > +\r
  95 > > +    if ((tags = notmuch_config_get_new_tags (config, &count)) == NULL)\r
  96 > > +   return;\r
  97 > \r
  98 > size_t != unsigned int on all platforms.  Please stick with one or the\r
  99 > other.  Note there are a few calls to fix here.\r
 100 \r
 101 That's a good catch. I will fix that one. Thanks a lot for the review, I\r
 102 really appreciate that!\r
 103 \r
 104   -- Jan\r
 105 \r