Re: [PATCH 2/2] emacs: Allow tagging regions in notmuch-tree

[David Bremner <david@tethera.net>]

Pierre Neidhardt <mail@ambrevar.xyz> 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.