99/1d471f8c7c910e762c56272c0c9ed6ac7c7b55

   1 Return-Path: <ethan.glasser.camp@gmail.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 56572431FBD\r
   6         for <notmuch@notmuchmail.org>; Sun, 18 Nov 2012 13:55:56 -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.799\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.799 tagged_above=-999 required=5\r
  12         tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1,\r
  13         FREEMAIL_FROM=0.001, RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
  14 Received: from olra.theworths.org ([127.0.0.1])\r
  15         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  16         with ESMTP id b-ajqx+NimOc for <notmuch@notmuchmail.org>;\r
  17         Sun, 18 Nov 2012 13:55:55 -0800 (PST)\r
  18 Received: from mail-qc0-f181.google.com (mail-qc0-f181.google.com\r
  19         [209.85.216.181]) (using TLSv1 with cipher RC4-SHA (128/128 bits))\r
  20         (No client certificate requested)\r
  21         by olra.theworths.org (Postfix) with ESMTPS id D4136431FB6\r
  22         for <notmuch@notmuchmail.org>; Sun, 18 Nov 2012 13:55:55 -0800 (PST)\r
  23 Received: by mail-qc0-f181.google.com with SMTP id x40so3164203qcp.26\r
  24         for <notmuch@notmuchmail.org>; Sun, 18 Nov 2012 13:55:55 -0800 (PST)\r
  25 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113;\r
  26         h=from:to:subject:in-reply-to:references:user-agent:date:message-id\r
  27         :mime-version:content-type;\r
  28         bh=/HKZfC1QBOVEMSc2efm5FR5i/r0NXl+pmoj8vWTLgao=;\r
  29         b=LOc3d6RSpqZJ4t2Zkkv1Uxv2ZBuYppevQZKYWuodFLLDv8t46pciltDQhUxri388BI\r
  30         Smo67Zn6QZ6clsS5RZba0WbeOvAL+Qv7hFq9KQYJ8rFGakvMmStlIKfjHu/Wiyq8U4p3\r
  31         A+GbGQ5aOQqqxlgr+NylCDlCP05R38uVYDhFDZeID4oEDjFFb8k/3OpUDxIf0Lho4VWB\r
  32         6MpxNzU2K9pwvho2pPg6AHu+Fsbse0Yziy838J7du+5cn/Wfa+cNGLYJutnguyrACJrk\r
  33         shzzBb5CVRreMBdNqbhGbjEkxYqpOhEQ05X4lJW3DSrhkqZ5l89ssJr7mXOV4i9cLdGK\r
  34         xKTg==\r
  35 Received: by 10.49.2.74 with SMTP id 10mr11599665qes.10.1353275755225;\r
  36         Sun, 18 Nov 2012 13:55:55 -0800 (PST)\r
  37 Received: from smtp.gmail.com ([66.114.71.21])\r
  38         by mx.google.com with ESMTPS id la6sm4366443qeb.8.2012.11.18.13.55.45\r
  39         (version=TLSv1/SSLv3 cipher=OTHER);\r
  40         Sun, 18 Nov 2012 13:55:46 -0800 (PST)\r
  41 From: Ethan Glasser-Camp <ethan.glasser.camp@gmail.com>\r
  42 To: david@tethera.net, notmuch@notmuchmail.org\r
  43 Subject: Re: Add new dump/restore format and batch tagging.\r
  44 In-Reply-To: <1353265498-3839-1-git-send-email-david@tethera.net>\r
  45 References: <1353265498-3839-1-git-send-email-david@tethera.net>\r
  46 User-Agent: Notmuch/0.14+45~g6ea9330 (http://notmuchmail.org) Emacs/24.1.1\r
  47         (x86_64-pc-linux-gnu)\r
  48 Date: Sun, 18 Nov 2012 16:55:43 -0500\r
  49 Message-ID: <87obiu1sps.fsf@betacantrips.com>\r
  50 MIME-Version: 1.0\r
  51 Content-Type: text/plain\r
  52 X-BeenThere: notmuch@notmuchmail.org\r
  53 X-Mailman-Version: 2.1.13\r
  54 Precedence: list\r
  55 List-Id: "Use and development of the notmuch mail system."\r
  56         <notmuch.notmuchmail.org>\r
  57 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  58         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  59 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  60 List-Post: <mailto:notmuch@notmuchmail.org>\r
  61 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  62 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  63         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  64 X-List-Received-Date: Sun, 18 Nov 2012 21:55:56 -0000\r
  65 \r
  66 david@tethera.net writes:\r
  67 \r
  68 > which was revied by Tomi and Ethan. I think I implemented their\r
  69 > suggestions.\r
  70 \r
  71 Actually, I don't think you implemented all of mine.\r
  72 \r
  73 - Patch 4 still has a subject line that ends in a period. I don't think\r
  74   this is mandatory for everyone but some people consider it best\r
  75   practice. You still have the spelling "seperate" (also, patch 7 has\r
  76   "seperated").\r
  77 \r
  78 - In patch 4, I still think this would look better if you switched the\r
  79   branches.\r
  80 \r
  81 +    if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) {\r
  82 +       _notmuch_message_add_term (message, "type", "mail");\r
  83 +    } else {\r
  84 +       return NOTMUCH_STATUS_DUPLICATE_MESSAGE_ID;\r
  85 +    }\r
  86 \r
  87 - In patch 5, I still think this looks funny:\r
  88 \r
  89 +       int num_tags = random () % (max_tags + 1);\r
  90 +       int this_mid_len = random () % message_id_len + 1;\r
  91 \r
  92 Additionally, I would like a check that message_id_len is reasonable\r
  93 (more than 1, say).\r
  94 \r
  95 - Patch 8:\r
  96 \r
  97 +int\r
  98 +parse_tag_stream (void *ctx,\r
  99 +                 notmuch_database_t *notmuch,\r
 100 +                 FILE *input,\r
 101 +                 tag_callback_t callback,\r
 102 +                 tag_op_flag_t flags,\r
 103 +                 volatile sig_atomic_t *interrupted);\r
 104 \r
 105 Am I going crazy, or does this function not get implemented?\r
 106 \r
 107 - Patch 11: DUMP_FORMAT can change from DUMP_FORMAT_BATCH_TAG to\r
 108   DUMP_FORMAT_SUP if a paren is anywhere on the first line. I'd prefer\r
 109   this only happens if we have DUMP_FORMAT_AUTO.\r
 110 \r
 111 You probably want to move the comment "Dump output is..." closer to the\r
 112 regex.\r
 113 \r
 114 I don't see why it's necessary to have this:\r
 115 \r
 116 +               query_string = query_string + 3;\r
 117 \r
 118 - Patch 13:\r
 119 \r
 120 +    cp /dev/null EXPECTED.$test_count\r
 121 +    notmuch dump --format=batch-tag -- from:cworth |\\r
 122 +        awk "{ print \"+$enc1 +$enc2 +$enc3 -- \" \$5 }" > EXPECTED.$test_count\r
 123 \r
 124 What's the purpose of the CP here? It just creates an empty file. You\r
 125 could do it with touch. Why even bother since you're going to fill it\r
 126 with stuff in a second? Actually, care to explain the dump and sed call?\r
 127 It looks like you're using this dump to encode the message IDs. If\r
 128 format=batch-tag skips a message for some reason or terminates early,\r
 129 the test won't fail.\r
 130 \r
 131 - Patch 16:\r
 132 \r
 133 +message-ids may contained arbitrary non-null characters. Note also\r
 134 \r
 135 I think this should be "may contain", or something else entirely if\r
 136 you're trying to describe past behavior of sup?\r
 137 \r
 138 Ethan\r