73/4e757074e6cb1530db833f67a365d0aeb1dd19

   1 Return-Path: <m.walters@qmul.ac.uk>\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 E2E37431FD7\r
   6         for <notmuch@notmuchmail.org>; Wed, 30 Oct 2013 11:48:24 -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: -1.098\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-1.098 tagged_above=-999 required=5\r
  12         tests=[DKIM_ADSP_CUSTOM_MED=0.001, FREEMAIL_FROM=0.001,\r
  13         NML_ADSP_CUSTOM_MED=1.2, RCVD_IN_DNSWL_MED=-2.3] 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 kMBSWFLanOkN for <notmuch@notmuchmail.org>;\r
  17         Wed, 30 Oct 2013 11:48:20 -0700 (PDT)\r
  18 Received: from mail2.qmul.ac.uk (mail2.qmul.ac.uk [138.37.6.6])\r
  19         (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits))\r
  20         (No client certificate requested)\r
  21         by olra.theworths.org (Postfix) with ESMTPS id E654A431FCF\r
  22         for <notmuch@notmuchmail.org>; Wed, 30 Oct 2013 11:48:19 -0700 (PDT)\r
  23 Received: from smtp.qmul.ac.uk ([138.37.6.40])\r
  24         by mail2.qmul.ac.uk with esmtp (Exim 4.71)\r
  25         (envelope-from <m.walters@qmul.ac.uk>)\r
  26         id 1VbaoO-0004LH-F9; Wed, 30 Oct 2013 18:48:15 +0000\r
  27 Received: from 93-97-24-31.zone5.bethere.co.uk ([93.97.24.31] helo=localhost)\r
  28         by smtp.qmul.ac.uk with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.71)\r
  29         (envelope-from <m.walters@qmul.ac.uk>)\r
  30         id 1VbaoO-0003gB-1p; Wed, 30 Oct 2013 18:48:12 +0000\r
  31 From: Mark Walters <markwalters1009@gmail.com>\r
  32 To: Tomi Ollila <tomi.ollila@iki.fi>, notmuch@notmuchmail.org\r
  33 Subject: Re: [PATCH 00/11] Move notmuch-tree to mainline\r
  34 In-Reply-To: <m2bo26eiy8.fsf@guru.guru-group.fi>\r
  35 References: <1383087338-10220-1-git-send-email-markwalters1009@gmail.com>\r
  36         <m2bo26eiy8.fsf@guru.guru-group.fi>\r
  37 User-Agent: Notmuch/0.16 (http://notmuchmail.org) Emacs/23.4.1\r
  38         (x86_64-pc-linux-gnu)\r
  39 Date: Wed, 30 Oct 2013 18:48:10 +0000\r
  40 Message-ID: <87d2mm8uj9.fsf@qmul.ac.uk>\r
  41 MIME-Version: 1.0\r
  42 Content-Type: text/plain; charset=us-ascii\r
  43 X-Sender-Host-Address: 93.97.24.31\r
  44 X-QM-SPAM-Info: Sender has good ham record.  :)\r
  45 X-QM-Body-MD5: 91e31d0220c74927fff2a9c926c6b196 (of first 20000 bytes)\r
  46 X-SpamAssassin-Score: 0.0\r
  47 X-SpamAssassin-SpamBar: /\r
  48 X-SpamAssassin-Report: The QM spam filters have analysed this message to\r
  49         determine if it is\r
  50         spam. We require at least 5.0 points to mark a message as spam.\r
  51         This message scored 0.0 points. Summary of the scoring: \r
  52         * 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail\r
  53         provider *      (markwalters1009[at]gmail.com)\r
  54         *  0.0 AWL AWL: From: address is in the auto white-list\r
  55 X-QM-Scan-Virus: ClamAV says the message is clean\r
  56 X-BeenThere: notmuch@notmuchmail.org\r
  57 X-Mailman-Version: 2.1.13\r
  58 Precedence: list\r
  59 List-Id: "Use and development of the notmuch mail system."\r
  60         <notmuch.notmuchmail.org>\r
  61 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  62         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  63 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  64 List-Post: <mailto:notmuch@notmuchmail.org>\r
  65 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  66 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  67         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  68 X-List-Received-Date: Wed, 30 Oct 2013 18:48:25 -0000\r
  69 \r
  70 \r
  71 Hi\r
  72 \r
  73 Thanks for the review\r
  74 \r
  75 On Wed, 30 Oct 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:\r
  76 > On Wed, Oct 30 2013, Mark Walters <markwalters1009@gmail.com> wrote:\r
  77 >\r
  78 >> This is a more complete version of the move of pick/tree to\r
  79 >> mainline. The previous version was at\r
  80 >> id:1382257699-29860-1-git-send-email-markwalters1009@gmail.com\r
  81 >\r
  82 > This series looks pretty good to me. IMO patches 1-6 are ready...\r
  83 >\r
  84 >> The previous version did not get the autoloading right: it all worked\r
  85 >> on my setup as I had stale files around in my tree which were loaded.\r
  86 >> This versions builds and tests pass at each stage on a fresh clone.\r
  87 >\r
  88 > You and Me seem to have different understanding what "autoload"ing means.\r
  89 \r
  90 Sorry about that; I just meant that it was automatically loaded when\r
  91 notmuch was loaded (that is the user no longer needs an explicit\r
  92 (require 'notmuch-tree) in their .emacs file.\r
  93 \r
  94 I would probably just stick with the require approach (to be consistent\r
  95 with show etc) unless you (or others) prefer the real autoload as you\r
  96 describe.\r
  97 \r
  98 I will fix the commit messages in the next version.\r
  99 \r
 100 Many thanks\r
 101 \r
 102 Mark\r
 103 \r
 104 \r
 105 > To me autoloading means using (autoload ...) function. In patch 7\r
 106 > ( id:1383087338-10220-8-git-send-email-markwalters1009@gmail.com )\r
 107 > (require 'notmuch-tree) is used.\r
 108 >\r
 109 > To me it is fine if this approach is taken, but then calling it autoloading\r
 110 > in commit message (in patches 7 and 8) is confusing.\r
 111 >\r
 112 > However, if (autoload ...) syntax were actually used, it could be done\r
 113 > the somewhat the following way:\r
 114 >\r
 115 > * Don't add (require 'notmuch-tree) in notmuch.el (Patch 7) \r
 116 >\r
 117 > * Add (autoload 'notmuch-tree-from-search-current-query "notmuch-tree" "docstring" t)\r
 118 >   to notmuch.el (maybe we can live without declare-function ?)\r
 119 >\r
 120 > * Add (autoload 'notmuch-tree "notmuch-tree" "docstring" t) to \r
 121 >   notmuch-lib.el.\r
 122 >\r
 123 > * Now Patch 9 is good as it is now.\r
 124 >\r
 125 > * Don't move the functions mentioned in Patch 9 from notmuch-tree.el to\r
 126 >   notmuch.el but write autoload (for notmuch-tree-from-search-thread) to\r
 127 >   notmuch.el\r
 128 >\r
 129 > * Now Patch 10 is good as it is now.\r
 130 >\r
 131 > * Leave notmuch-tree-from-show-current-query in notmuch-tree.el, and add\r
 132 >   autoload for it in notmuch-show.el. Now you can also drop \r
 133 >   (declare-function notmuch-tree ...) out from notmuch-show.el (Patch 11).\r
 134 >\r
 135 >\r
 136 > Tomi\r
 137 >\r
 138 >\r
 139 >> The diffstat at the bottom of this message is huge, so I include the\r
 140 >> rather more informative output from git diff -stat -M here.\r
 141 >>\r
 142 >> Apart from the first patch (which moves 80 lines from notmuch.el to notmuch-lib.el) the total is:\r
 143 >>  emacs/Makefile.local                               |    1 +\r
 144 >>  emacs/notmuch-lib.el                               |    1 +\r
 145 >>  emacs/notmuch-show.el                              |   10 ++++\r
 146 >>  {contrib/notmuch-pick => emacs}/notmuch-tree.el    |   47 ++-----------------\r
 147 >>  emacs/notmuch.el                                   |   16 +++++++\r
 148 >>  {contrib/notmuch-pick/test => test}/emacs-tree     |   13 -----\r
 149 >>  test/notmuch-test                                  |    1 +\r
 150 >>  .../tree.expected-output/notmuch-tree-show-window  |    0\r
 151 >>  .../notmuch-tree-single-thread                     |    0\r
 152 >>  .../tree.expected-output/notmuch-tree-tag-inbox    |    0\r
 153 >>  .../notmuch-tree-tag-inbox-tagged                  |    0\r
 154 >>  .../notmuch-tree-tag-inbox-thread-tagged           |    0\r
 155 >>  12 files changed, 34 insertions(+), 55 deletions(-)\r
 156 >>\r
 157 >>\r
 158 >> Obviously this clashes with the remap/help series at\r
 159 >> id:1382821479-23384-1-git-send-email-markwalters1009@gmail.com but\r
 160 >> rebasing either way would be easy.\r
 161 >>\r
 162 >> Best wishes\r
 163 >>\r
 164 >> Mark\r
 165 >>\r
 166 >>\r
 167 >>\r
 168 >>\r
 169 >>\r
 170 >> Mark Walters (11):\r
 171 >>   emacs: move notmuch-help to lib\r
 172 >>   emacs: tree: remove unneeded declarations\r
 173 >>   emacs: move notmuch-tree from contrib to mainline\r
 174 >>   emacs: add tree to the makefile\r
 175 >>   test: move emacs-tree test into mainline\r
 176 >>   emacs: tree: remove test for emacs from tree test\r
 177 >>   emacs: minimal change to autoload notmuch-tree\r
 178 >>   test: tree: remove require from tests\r
 179 >>   emacs: move search based tree functions to notmuch.el\r
 180 >>   emacs: add z to common keymap\r
 181 >>   emacs: move the show entry to tree into show.el\r
 182 >>\r
 183 >>  contrib/notmuch-pick/notmuch-tree.el               |  951 --------------------\r
 184 >>  contrib/notmuch-pick/test/emacs-tree               |  210 -----\r
 185 >>  .../tree.expected-output/notmuch-tree-show-window  |   40 -\r
 186 >>  .../notmuch-tree-single-thread                     |    6 -\r
 187 >>  .../tree.expected-output/notmuch-tree-tag-inbox    |   53 --\r
 188 >>  .../notmuch-tree-tag-inbox-tagged                  |   53 --\r
 189 >>  .../notmuch-tree-tag-inbox-thread-tagged           |   53 --\r
 190 >>  emacs/Makefile.local                               |    1 +\r
 191 >>  emacs/notmuch-lib.el                               |   88 ++\r
 192 >>  emacs/notmuch-show.el                              |   10 +\r
 193 >>  emacs/notmuch-tree.el                              |  914 +++++++++++++++++++\r
 194 >>  emacs/notmuch.el                                   |  103 +--\r
 195 >>  test/emacs-tree                                    |  197 ++++\r
 196 >>  test/notmuch-test                                  |    1 +\r
 197 >>  test/tree.expected-output/notmuch-tree-show-window |   40 +\r
 198 >>  .../notmuch-tree-single-thread                     |    6 +\r
 199 >>  test/tree.expected-output/notmuch-tree-tag-inbox   |   53 ++\r
 200 >>  .../notmuch-tree-tag-inbox-tagged                  |   53 ++\r
 201 >>  .../notmuch-tree-tag-inbox-thread-tagged           |   53 ++\r
 202 >>  19 files changed, 1432 insertions(+), 1453 deletions(-)\r
 203 >>  delete mode 100644 contrib/notmuch-pick/notmuch-tree.el\r
 204 >>  delete mode 100755 contrib/notmuch-pick/test/emacs-tree\r
 205 >>  delete mode 100644 contrib/notmuch-pick/test/tree.expected-output/notmuch-tree-show-window\r
 206 >>  delete mode 100644 contrib/notmuch-pick/test/tree.expected-output/notmuch-tree-single-thread\r
 207 >>  delete mode 100644 contrib/notmuch-pick/test/tree.expected-output/notmuch-tree-tag-inbox\r
 208 >>  delete mode 100644 contrib/notmuch-pick/test/tree.expected-output/notmuch-tree-tag-inbox-tagged\r
 209 >>  delete mode 100644 contrib/notmuch-pick/test/tree.expected-output/notmuch-tree-tag-inbox-thread-tagged\r
 210 >>  create mode 100644 emacs/notmuch-tree.el\r
 211 >>  create mode 100755 test/emacs-tree\r
 212 >>  create mode 100644 test/tree.expected-output/notmuch-tree-show-window\r
 213 >>  create mode 100644 test/tree.expected-output/notmuch-tree-single-thread\r
 214 >>  create mode 100644 test/tree.expected-output/notmuch-tree-tag-inbox\r
 215 >>  create mode 100644 test/tree.expected-output/notmuch-tree-tag-inbox-tagged\r
 216 >>  create mode 100644 test/tree.expected-output/notmuch-tree-tag-inbox-thread-tagged\r
 217 >>\r
 218 >> -- \r
 219 >> 1.7.9.1\r
 220 >>\r
 221 >> _______________________________________________\r
 222 >> notmuch mailing list\r
 223 >> notmuch@notmuchmail.org\r
 224 >> http://notmuchmail.org/mailman/listinfo/notmuch\r