5d/0f2f5f20720c82dc8888e25969a67122c22926

   1 Return-Path: <cworth@cworth.org>\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 70D8D431FD0\r
   6         for <notmuch@notmuchmail.org>; Mon, 16 May 2011 13:42:43 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: 0.01\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=0.01 tagged_above=-999 required=5\r
  12         tests=[T_MIME_NO_TEXT=0.01] 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 1jzTpsbxb2zh for <notmuch@notmuchmail.org>;\r
  16         Mon, 16 May 2011 13:42:42 -0700 (PDT)\r
  17 Received: from arlo.cworth.org (arlo.cworth.org [50.43.72.2])\r
  18         by olra.theworths.org (Postfix) with ESMTP id D6759431FB6\r
  19         for <notmuch@notmuchmail.org>; Mon, 16 May 2011 13:42:42 -0700 (PDT)\r
  20 Received: from yoom.home.cworth.org (localhost [127.0.0.1])\r
  21         by arlo.cworth.org (Postfix) with ESMTP id 2CE9929A056;\r
  22         Mon, 16 May 2011 13:42:41 -0700 (PDT)\r
  23 Received: by yoom.home.cworth.org (Postfix, from userid 1000)\r
  24         id DA64C254183; Mon, 16 May 2011 13:42:40 -0700 (PDT)\r
  25 From: Carl Worth <cworth@cworth.org>\r
  26 To: Jameson Graef Rollins <jrollins@finestructure.net>,\r
  27         Notmuch Mail <notmuch@notmuchmail.org>\r
  28 Subject: Re: release-candidate/0.6\r
  29 In-Reply-To: <87wrhvyqfn.fsf@servo.factory.finestructure.net>\r
  30 References: <8762pn7gth.fsf@servo.factory.finestructure.net>\r
  31         <874o4zczr8.fsf@yoom.home.cworth.org>\r
  32         <87wrhvyqfn.fsf@servo.factory.finestructure.net>\r
  33 User-Agent: Notmuch/0.5 (http://notmuchmail.org) Emacs/23.3.1\r
  34         (i486-pc-linux-gnu)\r
  35 Date: Mon, 16 May 2011 13:42:33 -0700\r
  36 Message-ID: <87r57ybcna.fsf@yoom.home.cworth.org>\r
  37 MIME-Version: 1.0\r
  38 Content-Type: multipart/signed; boundary="=-=-=";\r
  39         micalg=pgp-sha1; protocol="application/pgp-signature"\r
  40 Cc: David Edmondson <dme@dme.org>\r
  41 X-BeenThere: notmuch@notmuchmail.org\r
  42 X-Mailman-Version: 2.1.13\r
  43 Precedence: list\r
  44 List-Id: "Use and development of the notmuch mail system."\r
  45         <notmuch.notmuchmail.org>\r
  46 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  47         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  48 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  49 List-Post: <mailto:notmuch@notmuchmail.org>\r
  50 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  51 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  52         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  53 X-List-Received-Date: Mon, 16 May 2011 20:42:43 -0000\r
  54 \r
  55 --=-=-=\r
  56 Content-Transfer-Encoding: quoted-printable\r
  57 \r
  58 On Fri, 13 May 2011 01:07:08 -0700, Jameson Graef Rollins <jrollins@finestr=\r
  59 ucture.net> wrote:\r
  60 > Hi, Carl.  I went through dme's multipart patch series and cleaned\r
  61 > things up.\r
  62 ...\r
  63 > The result is the new\r
  64 >=20\r
  65 > release-candidate/0.6+mpmfix\r
  66 \r
  67 Thanks so much! This looks much better than before.\r
  68 \r
  69 I'm still hitting some snags quite early in the review process, (I'm\r
  70 hoping that real soon we'll be able to just start merging like crazy).\r
  71 \r
  72 Here at the things I've seen so far:\r
  73 \r
  74 d3e7415d "add argument to part format function to indicate initial part"\r
  75 \r
  76         This one fails to build due to a simple missing argument, (easy\r
  77         mistake with the recent splitting of patches).\r
  78 \r
  79 373f352b "Have output structure fully reflect message MIME structure"\r
  80 \r
  81         I'm not comfortable with this commit. Previously there was\r
  82         recursion in one function, (show_message_part), and afterwards\r
  83         there is duplicated code performing recursion in both\r
  84         format_part_text and format_part_json.\r
  85 \r
  86         Similarly, the code adds header formatting code that duplicates\r
  87         functionality in the existing format_headers_text and\r
  88         format_headers_json functions.\r
  89 \r
  90         Meanwhile, I still can't tell exactly what the behavioral change\r
  91         intended is. The commit message talks about "fully recursing"\r
  92         and "match[ing] the MIME structure of the message". Was it not\r
  93         fully recursing before? In what way did the output not match the\r
  94         MIME structure before?\r
  95 \r
  96         It would probably be easier to tell what was going on if the\r
  97         test suite were updated at the same time (or in a subsequent\r
  98         commit at least). As is, this commit introduces test suite\r
  99         failures, (re-ordering of MIME part ID numbers and then\r
 100         emacs-client confusion), which remain until commit 7ee6aaa1\r
 101 \r
 102         But what does the rest of the series really need from this\r
 103         commit? Is there some structural change to the json output aside\r
 104         from the part ID? If so, we're definitely missing a test for\r
 105         that directly, (other than the indirect testing we get with the\r
 106         emacs tests).\r
 107 \r
 108 28ab74e0 "clean up expected emacs output to reflect cleaner from lines intr=\r
 109 oduced in 78d6c196d90"\r
 110 \r
 111         This commit message refers to a stale (now non-existent) commit\r
 112         ID. I presume it's attempting to reference the commit with a\r
 113         message of "emacs: Show cleaner `From:' addresses in the summary\r
 114         line.".\r
 115 \r
 116         Granted, it's hard to keep commit IDs valid in a branch that's\r
 117         getting continually rebuilt. One fix is to not use the commit\r
 118         identifiers. Another help would be to have the test-suite\r
 119         cleanups occurring more closely after the commits that changed\r
 120         things.\r
 121 \r
 122 I'm happy to help work on cleaning up some of these issues if that would\r
 123 be useful. Let me know.\r
 124 \r
 125 =2DCarl\r
 126 \r
 127 --=-=-=\r
 128 Content-Type: application/pgp-signature\r
 129 \r
 130 -----BEGIN PGP SIGNATURE-----\r
 131 Version: GnuPG v1.4.11 (GNU/Linux)\r
 132 \r
 133 iEYEARECAAYFAk3RjDkACgkQ6JDdNq8qSWjPtQCfevHcj/kFsB5dLgO6WQvNFcIw\r
 134 VYEAni2vjNXfsGOp/cz0ClEyuHFcvK5e\r
 135 =pugM\r
 136 -----END PGP SIGNATURE-----\r
 137 --=-=-=--\r