From: Tomi Ollila Date: Wed, 30 Oct 2013 18:01:51 +0000 (+0200) Subject: Re: [PATCH 00/11] Move notmuch-tree to mainline X-Git-Url: http://git.tremily.us/?a=commitdiff_plain;h=0b776c200a7bc7966fdd26a7ab2c0c7727c744c4;p=notmuch-archives.git Re: [PATCH 00/11] Move notmuch-tree to mainline --- diff --git a/7a/deecce83732fde299bba1a46e4448d35b1b80e b/7a/deecce83732fde299bba1a46e4448d35b1b80e new file mode 100644 index 000000000..e343537db --- /dev/null +++ b/7a/deecce83732fde299bba1a46e4448d35b1b80e @@ -0,0 +1,183 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id 58730431FD6 + for ; Wed, 30 Oct 2013 11:02:08 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: 0 +X-Spam-Level: +X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none] + autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id dUX81p3ZL+sp for ; + Wed, 30 Oct 2013 11:02:00 -0700 (PDT) +Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34]) + by olra.theworths.org (Postfix) with ESMTP id BBB60431FCF + for ; Wed, 30 Oct 2013 11:01:59 -0700 (PDT) +Received: from guru.guru-group.fi (localhost [IPv6:::1]) + by guru.guru-group.fi (Postfix) with ESMTP id 7E11C1000B2; + Wed, 30 Oct 2013 20:01:51 +0200 (EET) +From: Tomi Ollila +To: Mark Walters , notmuch@notmuchmail.org +Subject: Re: [PATCH 00/11] Move notmuch-tree to mainline +In-Reply-To: <1383087338-10220-1-git-send-email-markwalters1009@gmail.com> +References: <1383087338-10220-1-git-send-email-markwalters1009@gmail.com> +User-Agent: Notmuch/0.16+119~g219c55f (http://notmuchmail.org) Emacs/24.3.1 + (x86_64-unknown-linux-gnu) +X-Face: HhBM'cA~ +MIME-Version: 1.0 +Content-Type: text/plain +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +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: Wed, 30 Oct 2013 18:02:08 -0000 + +On Wed, Oct 30 2013, Mark Walters wrote: + +> This is a more complete version of the move of pick/tree to +> mainline. The previous version was at +> id:1382257699-29860-1-git-send-email-markwalters1009@gmail.com + +This series looks pretty good to me. IMO patches 1-6 are ready... + +> The previous version did not get the autoloading right: it all worked +> on my setup as I had stale files around in my tree which were loaded. +> This versions builds and tests pass at each stage on a fresh clone. + +You and Me seem to have different understanding what "autoload"ing means. +To me autoloading means using (autoload ...) function. In patch 7 +( id:1383087338-10220-8-git-send-email-markwalters1009@gmail.com ) +(require 'notmuch-tree) is used. + +To me it is fine if this approach is taken, but then calling it autoloading +in commit message (in patches 7 and 8) is confusing. + +However, if (autoload ...) syntax were actually used, it could be done +the somewhat the following way: + +* Don't add (require 'notmuch-tree) in notmuch.el (Patch 7) + +* Add (autoload 'notmuch-tree-from-search-current-query "notmuch-tree" "docstring" t) + to notmuch.el (maybe we can live without declare-function ?) + +* Add (autoload 'notmuch-tree "notmuch-tree" "docstring" t) to + notmuch-lib.el. + +* Now Patch 9 is good as it is now. + +* Don't move the functions mentioned in Patch 9 from notmuch-tree.el to + notmuch.el but write autoload (for notmuch-tree-from-search-thread) to + notmuch.el + +* Now Patch 10 is good as it is now. + +* Leave notmuch-tree-from-show-current-query in notmuch-tree.el, and add + autoload for it in notmuch-show.el. Now you can also drop + (declare-function notmuch-tree ...) out from notmuch-show.el (Patch 11). + + +Tomi + + +> The diffstat at the bottom of this message is huge, so I include the +> rather more informative output from git diff -stat -M here. +> +> Apart from the first patch (which moves 80 lines from notmuch.el to notmuch-lib.el) the total is: +> emacs/Makefile.local | 1 + +> emacs/notmuch-lib.el | 1 + +> emacs/notmuch-show.el | 10 ++++ +> {contrib/notmuch-pick => emacs}/notmuch-tree.el | 47 ++----------------- +> emacs/notmuch.el | 16 +++++++ +> {contrib/notmuch-pick/test => test}/emacs-tree | 13 ----- +> test/notmuch-test | 1 + +> .../tree.expected-output/notmuch-tree-show-window | 0 +> .../notmuch-tree-single-thread | 0 +> .../tree.expected-output/notmuch-tree-tag-inbox | 0 +> .../notmuch-tree-tag-inbox-tagged | 0 +> .../notmuch-tree-tag-inbox-thread-tagged | 0 +> 12 files changed, 34 insertions(+), 55 deletions(-) +> +> +> Obviously this clashes with the remap/help series at +> id:1382821479-23384-1-git-send-email-markwalters1009@gmail.com but +> rebasing either way would be easy. +> +> Best wishes +> +> Mark +> +> +> +> +> +> Mark Walters (11): +> emacs: move notmuch-help to lib +> emacs: tree: remove unneeded declarations +> emacs: move notmuch-tree from contrib to mainline +> emacs: add tree to the makefile +> test: move emacs-tree test into mainline +> emacs: tree: remove test for emacs from tree test +> emacs: minimal change to autoload notmuch-tree +> test: tree: remove require from tests +> emacs: move search based tree functions to notmuch.el +> emacs: add z to common keymap +> emacs: move the show entry to tree into show.el +> +> contrib/notmuch-pick/notmuch-tree.el | 951 -------------------- +> contrib/notmuch-pick/test/emacs-tree | 210 ----- +> .../tree.expected-output/notmuch-tree-show-window | 40 - +> .../notmuch-tree-single-thread | 6 - +> .../tree.expected-output/notmuch-tree-tag-inbox | 53 -- +> .../notmuch-tree-tag-inbox-tagged | 53 -- +> .../notmuch-tree-tag-inbox-thread-tagged | 53 -- +> emacs/Makefile.local | 1 + +> emacs/notmuch-lib.el | 88 ++ +> emacs/notmuch-show.el | 10 + +> emacs/notmuch-tree.el | 914 +++++++++++++++++++ +> emacs/notmuch.el | 103 +-- +> test/emacs-tree | 197 ++++ +> test/notmuch-test | 1 + +> test/tree.expected-output/notmuch-tree-show-window | 40 + +> .../notmuch-tree-single-thread | 6 + +> test/tree.expected-output/notmuch-tree-tag-inbox | 53 ++ +> .../notmuch-tree-tag-inbox-tagged | 53 ++ +> .../notmuch-tree-tag-inbox-thread-tagged | 53 ++ +> 19 files changed, 1432 insertions(+), 1453 deletions(-) +> delete mode 100644 contrib/notmuch-pick/notmuch-tree.el +> delete mode 100755 contrib/notmuch-pick/test/emacs-tree +> delete mode 100644 contrib/notmuch-pick/test/tree.expected-output/notmuch-tree-show-window +> delete mode 100644 contrib/notmuch-pick/test/tree.expected-output/notmuch-tree-single-thread +> delete mode 100644 contrib/notmuch-pick/test/tree.expected-output/notmuch-tree-tag-inbox +> delete mode 100644 contrib/notmuch-pick/test/tree.expected-output/notmuch-tree-tag-inbox-tagged +> delete mode 100644 contrib/notmuch-pick/test/tree.expected-output/notmuch-tree-tag-inbox-thread-tagged +> create mode 100644 emacs/notmuch-tree.el +> create mode 100755 test/emacs-tree +> create mode 100644 test/tree.expected-output/notmuch-tree-show-window +> create mode 100644 test/tree.expected-output/notmuch-tree-single-thread +> create mode 100644 test/tree.expected-output/notmuch-tree-tag-inbox +> create mode 100644 test/tree.expected-output/notmuch-tree-tag-inbox-tagged +> create mode 100644 test/tree.expected-output/notmuch-tree-tag-inbox-thread-tagged +> +> -- +> 1.7.9.1 +> +> _______________________________________________ +> notmuch mailing list +> notmuch@notmuchmail.org +> http://notmuchmail.org/mailman/listinfo/notmuch