3e/a902df136832b2dff0a17e15907f5b40926d06

   1 Return-Path: <wking@tremily.us>\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 19DC1431FAF\r
   6         for <notmuch@notmuchmail.org>; Wed,  6 Aug 2014 16:21:38 -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.1\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.1 tagged_above=-999 required=5\r
  12         tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1,\r
  13         RCVD_IN_DNSWL_NONE=-0.0001] 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 tGHyp2wqZOC6 for <notmuch@notmuchmail.org>;\r
  17         Wed,  6 Aug 2014 16:21:34 -0700 (PDT)\r
  18 Received: from qmta08.emeryville.ca.mail.comcast.net\r
  19         (qmta08.emeryville.ca.mail.comcast.net [76.96.30.80])\r
  20         by olra.theworths.org (Postfix) with ESMTP id 06F39431FAE\r
  21         for <notmuch@notmuchmail.org>; Wed,  6 Aug 2014 16:21:33 -0700 (PDT)\r
  22 Received: from omta17.emeryville.ca.mail.comcast.net ([76.96.30.73])\r
  23         by qmta08.emeryville.ca.mail.comcast.net with comcast\r
  24         id bas61o0021afHeLA8bMY6H; Wed, 06 Aug 2014 23:21:32 +0000\r
  25 Received: from odin.tremily.us ([24.18.63.50])\r
  26         by omta17.emeryville.ca.mail.comcast.net with comcast\r
  27         id bbMX1o00P152l3L8dbMYDM; Wed, 06 Aug 2014 23:21:32 +0000\r
  28 Received: by odin.tremily.us (Postfix, from userid 1000)\r
  29         id 6A4C512BD95C; Wed,  6 Aug 2014 16:21:31 -0700 (PDT)\r
  30 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tremily.us; s=odin;\r
  31         t=1407367291; bh=6KhD8cv7Gv5cvBdTmHDN5v6dULkD2duvwQ7FTfc2ouA=;\r
  32         h=Date:From:To:Cc:Subject:References:In-Reply-To;\r
  33         b=ZN25iZCPAAtSECzW+EOgGtm9p+FYV5mG1GbPan2i7/wxSeylcZccIp1diRogqUMYH\r
  34         WJbH8Y2sODzDIheACbHjp7/DH5+t23BRBugqM6z/3RPQ2+fCQ4/fPDQo45d9VPS9NL\r
  35         nVR84CgjUFKRwZAcu/HxIxx7ltA7du9HgNbBm/EE=\r
  36 Date: Wed, 6 Aug 2014 16:21:31 -0700\r
  37 From: "W. Trevor King" <wking@tremily.us>\r
  38 To: David Bremner <bremner@debian.org>\r
  39 Subject: Re: [PATCH v3] nmbug: Translate to Python\r
  40 Message-ID: <20140806232131.GH31313@odin.tremily.us>\r
  41 References:\r
  42  <84447a0ed48412e1587761d560d18cb5affd4f66.1405897133.git.wking@tremily.us>\r
  43         <878un36bd5.fsf@maritornes.cs.unb.ca>\r
  44 MIME-Version: 1.0\r
  45 Content-Type: multipart/signed; micalg=pgp-sha1;\r
  46         protocol="application/pgp-signature"; boundary="ewQ5hdP4CtoTt3oD"\r
  47 Content-Disposition: inline\r
  48 In-Reply-To: <878un36bd5.fsf@maritornes.cs.unb.ca>\r
  49 OpenPGP: id=39A2F3FA2AB17E5D8764F388FC29BDCDF15F5BE8;\r
  50         url=http://tremily.us/pubkey.txt\r
  51 User-Agent: Mutt/1.5.23 (2014-03-12)\r
  52 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=comcast.net;\r
  53         s=q20140121; t=1407367292;\r
  54         bh=JXAFOANff+cuqgs9iFvc4Yn7+eq/YsqzPVQnyVXUChY=;\r
  55         h=Received:Received:Received:Date:From:To:Subject:Message-ID:\r
  56         MIME-Version:Content-Type;\r
  57         b=oCRl+rW6Fo1Bo1YHorirThJQDj9Xg7lZAEyIcdrnfrwvSYbNh3bonoezLsPpCYDGp\r
  58         Y4DIjlXoxETUEbQWnrmMoTajaRO41NLRq9nclRUt2WnoH+AxcrAevD3PIwP4rQEAVR\r
  59         RaJafv7nvynB+B6eoQaLkQ2vRub3cb3ZuxU7zaT7v8+eGw38/Bm2waqMSsbozb05Ws\r
  60         hPNhB3m7Fwwqw1zGWq3EWFVxxn2qgiLW9PSGnahloSAnJvmEh8PNkjcMSRMwIuB+4L\r
  61         4oiiUNIk3/qnuvhad8WCZsFQrgYGSIYXfUS3Ja6husk6DaAd3shNz0+jjguZZBl7p/\r
  62         mzfnrGoxMNydg==\r
  63 Cc: notmuch@notmuchmail.org\r
  64 X-BeenThere: notmuch@notmuchmail.org\r
  65 X-Mailman-Version: 2.1.13\r
  66 Precedence: list\r
  67 List-Id: "Use and development of the notmuch mail system."\r
  68         <notmuch.notmuchmail.org>\r
  69 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  70         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  71 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  72 List-Post: <mailto:notmuch@notmuchmail.org>\r
  73 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  74 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  75         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  76 X-List-Received-Date: Wed, 06 Aug 2014 23:21:38 -0000\r
  77 \r
  78 \r
  79 --ewQ5hdP4CtoTt3oD\r
  80 Content-Type: text/plain; charset=utf-8\r
  81 Content-Disposition: inline\r
  82 Content-Transfer-Encoding: quoted-printable\r
  83 \r
  84 On Mon, Aug 04, 2014 at 09:14:46PM -0300, David Bremner wrote:\r
  85 > W. Trevor King writes:\r
  86 > > * Commands are no longer split into "most common", "other useful",\r
  87 > >   and "less common" sets.  If we need something like this, I'd\r
  88 > >   prefer workflow examples highlighting common commands in the\r
  89 > >   module docstring (available with 'nmbug --help').\r
  90 >=20\r
  91 > I don't feel strongly about this, but I remember implementing it by\r
  92 > request in the first version. OTOH, I think you shortened up the\r
  93 > main help string when you split it.\r
  94 \r
  95 I didn't intentionally remove any information.  I think it's just\r
  96 shorter because command-specific details are now in the\r
  97 command-specific docstring/help.\r
  98 \r
  99 > We may want to think about a seperate man page as a follow project.\r
 100 \r
 101 Works for me, but I think it would look a lot like the wiki page [1].\r
 102 \r
 103 > One thing I did notice is that there is no hint to call nmbug\r
 104 > {command} --help in the main docstring.\r
 105 \r
 106 I'll add that in v4.\r
 107 \r
 108 > > +#!/usr/bin/env python\r
 109 > > +# Copyright (c) 2011 David Bremner\r
 110 > > +# License: same as notmuch\r
 111 >=20\r
 112 > You should add your self, update the date, and probably explicitly\r
 113 > state the license, as in Carl's patch for nmbug-status.\r
 114 \r
 115 Will do in v4.\r
 116 \r
 117 > > +__version__ =3D '0.2'\r
 118 >=20\r
 119 > Do we need/want a version distinct from that of notmuch?\r
 120 \r
 121 nmbug is very loosely bound to the notmuch core.  To me it feels like\r
 122 a separate project that happens to share the same version control\r
 123 repository.  I'm happy to synchronize versions, but then we have to\r
 124 remember to bump the nmbug version for each notmuch release.\r
 125 \r
 126 > > +def _hex_quote(string, safe=3D'+@=3D:,'):\r
 127 >=20\r
 128 > I'm not sure I really understand what makes a function/variable\r
 129 > "private" and hence prefixed with _ in your translation.\r
 130 \r
 131 The public interface is what I thought was reasonably stable for folks\r
 132 who want to call nmbug as a Python library.  It's basically the\r
 133 command-line functionality, with a few other helpers that seemed\r
 134 important enough to be worth preserving.  If we keep a separate nmbug\r
 135 version, I'd cut major releases for anything that broke compatibility\r
 136 on a public function.  I don't need to use nmbug as a library myself,\r
 137 so this might all be over-engineered ;).\r
 138 \r
 139 > > +    status, tree, stderr =3D _git(\r
 140 >=20\r
 141 > as a non-native speaker of python, I find this a bit hard to read.\r
 142 > How about adding some parens to make the multiple return more clear,\r
 143 > so\r
 144 >=20\r
 145 >     (status, tree, stderr) =3D _git(\r
 146 \r
 147 That's legal, but I rarely see the parenthesized version in the wild.\r
 148 For examples showing the unparenthesized version, see [2,3].\r
 149 Parentheses are optional for Python tuples [4], so you'd only want\r
 150 them if =E2=80=98=3D=E2=80=99 had a higher precedence than =E2=80=98,=E2=80=\r
 151 =99.  That's my argument for\r
 152 the unparenthesized version, but feel free to overrule me ;).\r
 153 \r
 154 > I did notice that merging was noticably noisier than I remembered.=20\r
 155 >=20\r
 156 > > +    output =3D _collections.defaultdict(\r
 157 > > +        lambda : _collections.defaultdict( # {tag: status_string}\r
 158 > > +            lambda : ' '))  # default local status\r
 159 >=20\r
 160 > The initial comment is confusing (to me) because it looks like code.\r
 161 > The two layers of defaultdict are a bit confusing too; it would help\r
 162 > to mention the key of the outer dictionary.\r
 163 \r
 164 In v4, I'll use a leading comment for the whole structure, instead of\r
 165 interleaving the comments.  I'll also mention explicitly that the\r
 166 outer layer is keyed by message id.\r
 167 \r
 168 > I guess it makes sense to go for a relatively straight translation\r
 169 > as a first step; I did wonder about whether using the python\r
 170 > bindings to notmuch would speed things up.  Any ideas about how to\r
 171 > even figure out where the bottlenecks are?\r
 172 \r
 173 You could profile the Python script [5].  With my usual workflow, the\r
 174 existing implementation isn't generating too many subprocesses.\r
 175 Loading the database is probably slow though, so I'd expect reasonable\r
 176 gains everywhere we call notmuch more than once.  I expect checkout\r
 177 out working directories to also be slow, for the few command that do\r
 178 that on behalf of Git.\r
 179 \r
 180 Cheers,\r
 181 Trevor\r
 182 \r
 183 \r
 184 [1]: http://notmuchmail.org/nmbug/\r
 185 [2]: https://docs.python.org/3.4/tutorial/introduction.html#first-steps-tow=\r
 186 ards-programming\r
 187 [3]: http://legacy.python.org/dev/peps/pep-3132/\r
 188 [4]: https://docs.python.org/3.4/reference/expressions.html#parenthesized-f=\r
 189 orms\r
 190 [5]: https://docs.python.org/3.4/library/profile.html\r
 191 \r
 192 --=20\r
 193 This email may be signed or encrypted with GnuPG (http://www.gnupg.org).\r
 194 For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy\r
 195 \r
 196 --ewQ5hdP4CtoTt3oD\r
 197 Content-Type: application/pgp-signature; name="signature.asc"\r
 198 Content-Description: OpenPGP digital signature\r
 199 \r
 200 -----BEGIN PGP SIGNATURE-----\r
 201 Version: GnuPG v2\r
 202 \r
 203 iQIcBAEBAgAGBQJT4rh5AAoJEG8/JgBt8ol8Vy0P/2kCLd5L+IERhLY0GO8a9CQD\r
 204 IazSnIzlMtYmeN97VM85zimoLnbhxnB7ATIGNk5B2tT43nB/t1Op2rd9ZqEzNExL\r
 205 UcCokyTxDFvyfGtV1JqPA1hwgHcxr8ghGkA56NRxYCah9DixMd0zsmTbPaDfYuJq\r
 206 S/if3LtByeTcD8Usgy9ZdU8A87ZR8Hrb+bOtWOr9OGFq38cNx7ymx9+OdNhi+KKU\r
 207 ZHGasavcqxuGC5HEMkmJh0vpsOpJw+gVEuaNKnW29jG/vZnu5FVloH2YXfifdjDu\r
 208 OLJ7bHhKZQ2uUGwdmX/2FViUSwW4/+j/YtkxKmJ9g2ekt5ro6PWuYnwVJlo00aZY\r
 209 Vh2XnidzogjrpEkfZkeMyozIondW9tGLzx4ZJc52/jb0quMTdlIQFBuqb0vjnKp7\r
 210 mhuifQZqabqOZIw6wuknThTWVOADqhgmb8oEEOMPEnG3V5fFljYAa8mMln8lIkpW\r
 211 /5tCw3TeOlubk9EnEhRy0Y0hX9Qfdi9dPk6t63CMiT4hk7+No97WYEBK+CvPmby6\r
 212 aZcSQr68E2zOYk8M6TPIv4PmyJc0r88M/BGPoj1hPw9B2wuKKzx3RW8JlvdjHJS/\r
 213 0rG++DgCfHAXr4AxuwKy2ltjhiSJDlWC9054CNrnNhfdvl5N0rQJJ9PxtbJtJMFJ\r
 214 xMzRS4BP2lBK6uemj19x\r
 215 =DaTM\r
 216 -----END PGP SIGNATURE-----\r
 217 \r
 218 --ewQ5hdP4CtoTt3oD--\r