From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by arlo.cworth.org (Postfix) with ESMTP id BFDB26DE1040 for ; Sat, 25 May 2019 04:13:55 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at cworth.org X-Spam-Flag: NO X-Spam-Score: -0.04 X-Spam-Level: X-Spam-Status: No, score=-0.04 tagged_above=-999 required=5 tests=[AWL=-0.039, SPF_PASS=-0.001] autolearn=disabled Received: from arlo.cworth.org ([127.0.0.1]) by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id u68la8ZaR_rx for ; Sat, 25 May 2019 04:13:54 -0700 (PDT) Received: from fethera.tethera.net (fethera.tethera.net [198.245.60.197]) by arlo.cworth.org (Postfix) with ESMTPS id 7643B6DE0F34 for ; Sat, 25 May 2019 04:13:54 -0700 (PDT) Received: from remotemail by fethera.tethera.net with local (Exim 4.89) (envelope-from ) id 1hUUca-0000uM-0n; Sat, 25 May 2019 07:13:52 -0400 Received: (nullmailer pid 4100 invoked by uid 1000); Sat, 25 May 2019 11:13:57 -0000 From: David Bremner To: Pierre Neidhardt , notmuch@notmuchmail.org Subject: Re: [PATCH 2/2] emacs: Allow tagging regions in notmuch-tree In-Reply-To: <87d0klz5di.fsf@ambrevar.xyz> References: <20190409164712.13198-1-mail@ambrevar.xyz> <20190409164745.13497-1-mail@ambrevar.xyz> <878svhfbx0.fsf@tethera.net> <87d0klz5di.fsf@ambrevar.xyz> Date: Sat, 25 May 2019 08:13:57 -0300 Message-ID: <87v9xyn5vu.fsf@tethera.net> MIME-Version: 1.0 Content-Type: text/plain X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 25 May 2019 11:13:55 -0000 Pierre Neidhardt writes: Thanks for the updated patches. > >> 3) We generally want at least one test for a new >> feature. test/T460-emacs-tree.sh has the existing tree related >> tests. Again, if you need help with the test suite, let us know. > > I've added a test, but I wasn't able to run it on Guix > (https://guix.info). It fails like this: > > --8<---------------cut here---------------start------------->8--- >> guix environment notmuch -- /home/ambrevar/.local/share/emacs/site-lisp/notmuch/test/T460-emacs-tree.sh > guix environment: error: execlp: No such file or directory: "/home/ambrevar/.local/share/emacs/site-lisp/notmuch/test/T460-emacs-tree.sh" > --8<---------------cut here---------------end--------------->8--- I can't really help you with Guix, but I suggest setting up some environment where you can run things in an interactive shell. In particular that error seems to be claiming the test file doesn't exist, which would be easy to debug in an interactive shell. > >> 4) Please use notmuch-tree-- as a prefix for new private symbols that >> users should not rely on. I'm not sure about which symbols that applies >> to here. > > I've mirrored the implementation of search-mode. > There are indeed a few functions that are only used locally, but so are > they in notmuch.el, so I guess this should be part of another patch. > For better or for worse, our standards are higher for new code. Since (as you have just observed) minor style problems in existing code tend to linger unfixed. I assume you are not using git-send-email because it's difficult for you; it's not that a big of a deal, although we do prefer series generated git-send-email for reviewing.