ae/e1d867c26f20fe0c03839d7ef73adabbb1a4e3

   1 Return-Path: <david@tethera.net>\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 5E201431FBC\r
   6         for <notmuch@notmuchmail.org>; Mon,  4 Aug 2014 17:14:58 -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\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none]\r
  12         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 lusK2EocsODv for <notmuch@notmuchmail.org>;\r
  16         Mon,  4 Aug 2014 17:14:52 -0700 (PDT)\r
  17 Received: from yantan.tethera.net (yantan.tethera.net [199.188.72.155])\r
  18         (using TLSv1 with cipher DHE-RSA-AES128-SHA (128/128 bits))\r
  19         (No client certificate requested)\r
  20         by olra.theworths.org (Postfix) with ESMTPS id A70CF431FAE\r
  21         for <notmuch@notmuchmail.org>; Mon,  4 Aug 2014 17:14:52 -0700 (PDT)\r
  22 Received: from remotemail by yantan.tethera.net with local (Exim 4.80)\r
  23         (envelope-from <david@tethera.net>)\r
  24         id 1XESOx-0000KB-8O; Mon, 04 Aug 2014 21:14:51 -0300\r
  25 Received: (nullmailer pid 9215 invoked by uid 1000); Tue, 05 Aug 2014\r
  26         00:14:46 -0000\r
  27 From: David Bremner <bremner@debian.org>\r
  28 To: "W. Trevor King" <wking@tremily.us>, notmuch@notmuchmail.org\r
  29 Subject: Re: [PATCH v3] nmbug: Translate to Python\r
  30 In-Reply-To:\r
  31  <84447a0ed48412e1587761d560d18cb5affd4f66.1405897133.git.wking@tremily.us>\r
  32 References:\r
  33  <84447a0ed48412e1587761d560d18cb5affd4f66.1405897133.git.wking@tremily.us>\r
  34 User-Agent: Notmuch/0.18.1+45~gf47eeac (http://notmuchmail.org) Emacs/24.3.1\r
  35         (x86_64-pc-linux-gnu)\r
  36 Date: Mon, 04 Aug 2014 21:14:46 -0300\r
  37 Message-ID: <878un36bd5.fsf@maritornes.cs.unb.ca>\r
  38 MIME-Version: 1.0\r
  39 Content-Type: text/plain\r
  40 X-BeenThere: notmuch@notmuchmail.org\r
  41 X-Mailman-Version: 2.1.13\r
  42 Precedence: list\r
  43 List-Id: "Use and development of the notmuch mail system."\r
  44         <notmuch.notmuchmail.org>\r
  45 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  46         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  47 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  48 List-Post: <mailto:notmuch@notmuchmail.org>\r
  49 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  50 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  51         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  52 X-List-Received-Date: Tue, 05 Aug 2014 00:14:58 -0000\r
  53 \r
  54 "W. Trevor King" <wking@tremily.us> writes:\r
  55 \r
  56 \r
  57 > * Commands are no longer split into "most common", "other useful", and\r
  58 >   "less common" sets.  If we need something like this, I'd prefer\r
  59 >   workflow examples highlighting common commands in the module\r
  60 >   docstring (available with 'nmbug --help').\r
  61 >\r
  62 \r
  63 I don't feel strongly about this, but I remember implementing it by\r
  64 request in the first version. OTOH, I think you shortened up the main\r
  65 help string when you split it.  We may want to think about a seperate\r
  66 man page as a follow project.\r
  67 \r
  68 One thing I did notice is that there is no hint to call \r
  69 nmbug {command} --help in the main docstring.\r
  70 \r
  71 \r
  72 > +#!/usr/bin/env python\r
  73 > +# Copyright (c) 2011 David Bremner\r
  74 > +# License: same as notmuch\r
  75 \r
  76 You should add your self, update the date, and probably explicitly \r
  77 state the license, as in Carl's patch for nmbug-status.\r
  78 \r
  79 \r
  80 > +__version__ = '0.2'\r
  81 \r
  82 Do we need/want a version distinct from that of notmuch?\r
  83 \r
  84 \r
  85 > +def _hex_quote(string, safe='+@=:,'):\r
  86 \r
  87 I'm not sure I really understand what makes a function/variable\r
  88 "private" and hence prefixed with _ in your translation.\r
  89 \r
  90 \r
  91 > +    status, tree, stderr = _git(\r
  92 \r
  93 as a non-native speaker of python, I find this a bit hard to read.  How\r
  94 about adding some parens to make the multiple return more clear, so\r
  95 \r
  96     (status, tree, stderr) = _git(\r
  97 \r
  98 \r
  99 > +        for id, tags in status['deleted'].items():\r
 100 same comment here.\r
 101 \r
 102 > +\r
 103 > +def merge(reference='@{upstream}'):\r
 104 > +    """\r
 105 \r
 106 I did notice that merging was noticably noisier than I remembered. \r
 107 \r
 108 \r
 109 > +    output = _collections.defaultdict(\r
 110 > +        lambda : _collections.defaultdict( # {tag: status_string}\r
 111 > +            lambda : ' '))  # default local status\r
 112 \r
 113 The initial comment is confusing (to me) because it looks like code.\r
 114 The two layers of defaultdict are a bit confusing too; it would help to\r
 115 mention the key of the outer dictionary.\r
 116 \r
 117 I guess it makes sense to go for a relatively straight translation as a\r
 118 first step; I did wonder about whether using the python bindings to\r
 119 notmuch would speed things up.  Any ideas about how to even figure out\r
 120 where the bottlenecks are?\r
 121 \r
 122 d\r
 123 \r
 124 \r
 125 \r