Re: [PATCH] emacs: globally replace non-branching (if COND (progn ... with (when ...
On Mon, 30 Jan 2012 09:23:40 +, David Edmondson d...@dme.org wrote: On Sat, 28 Jan 2012 13:14:45 -0400, David Bremner da...@tethera.net wrote: On Sat, 28 Jan 2012 14:55:22 +0200, Jani Nikula j...@nikula.org wrote: On Jan 28, 2012 2:41 PM, David Bremner da...@tethera.net wrote: Sometimes someone (Dmitry?) sent patches that separated a small functional change, and the big non-functional indentation change it caused, separately. Would you prefer (or tolerate ;) that style? Hmm, that might be nicer, I'm not 100% sure. I wouldn't say it's mandatory for a patch like this (and I'd say other peoples views on what's easy to review are at least as important as mine here). Each patch should be valid in the repository without any following patches (preceding are obviously okay). Incorrect indentation would disqualify a patch from being 'valid', so it shouldn't be accepted. +1. Indentation corrections should always be part of the same patch as the change(s) that invalidated the indentation in the first place. Spotting the *actual* (non-indentation) changes is a non-issue when using `diff-refine-hunk' (or a wrapper thereof [1]). ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch Peace -- Pieter [1] id:8739ax7jts@praet.org ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] emacs: globally replace non-branching (if COND (progn ... with (when ...
On Mon, 30 Jan 2012 23:34:12 -0400, David Bremner da...@tethera.net wrote: On Sat, 14 Jan 2012 10:17:18 +0100, Pieter Praet pie...@praet.org wrote: Less code, same results, without sacrificing readability. --- Sorry, it conflicts with Jamie's patches that I just pushed. Please don't apologize for doing the right thing :) Rebased patch follows. d Peace -- Pieter ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] emacs: globally replace non-branching (if COND (progn ... with (when ...
On Wed, 01 Feb 2012 14:46:19 +0100, Pieter Praet pie...@praet.org wrote: On Mon, 30 Jan 2012 09:23:40 +, David Edmondson d...@dme.org wrote: On Sat, 28 Jan 2012 13:14:45 -0400, David Bremner da...@tethera.net wrote: On Sat, 28 Jan 2012 14:55:22 +0200, Jani Nikula j...@nikula.org wrote: On Jan 28, 2012 2:41 PM, David Bremner da...@tethera.net wrote: Sometimes someone (Dmitry?) sent patches that separated a small functional change, and the big non-functional indentation change it caused, separately. Would you prefer (or tolerate ;) that style? Hmm, that might be nicer, I'm not 100% sure. I wouldn't say it's mandatory for a patch like this (and I'd say other peoples views on what's easy to review are at least as important as mine here). Each patch should be valid in the repository without any following patches (preceding are obviously okay). Incorrect indentation would disqualify a patch from being 'valid', so it shouldn't be accepted. +1. Indentation corrections should always be part of the same patch as the change(s) that invalidated the indentation in the first place. Spotting the *actual* (non-indentation) changes is a non-issue when using `diff-refine-hunk' (or a wrapper thereof [1]). Must... resist... getting... into... this... ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] emacs: globally replace non-branching (if COND (progn ... with (when ...
On Sat, 14 Jan 2012 10:17:18 +0100, Pieter Praet pie...@praet.org wrote: Less code, same results, without sacrificing readability. This looks OK, although the re-indenting makes these kind of changes painful to review (not that I'm suggesting we should re-indent, just some random complaining). d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] emacs: globally replace non-branching (if COND (progn ... with (when ...
On Jan 28, 2012 2:41 PM, David Bremner da...@tethera.net wrote: On Sat, 14 Jan 2012 10:17:18 +0100, Pieter Praet pie...@praet.org wrote: Less code, same results, without sacrificing readability. This looks OK, although the re-indenting makes these kind of changes painful to review (not that I'm suggesting we should re-indent, just some random complaining). Sometimes someone (Dmitry?) sent patches that separated a small functional change, and the big non-functional indentation change it caused, separately. Would you prefer (or tolerate ;) that style? d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch
Re: [PATCH] emacs: globally replace non-branching (if COND (progn ... with (when ...
On Sat, 28 Jan 2012 14:55:22 +0200, Jani Nikula j...@nikula.org wrote: On Jan 28, 2012 2:41 PM, David Bremner da...@tethera.net wrote: Sometimes someone (Dmitry?) sent patches that separated a small functional change, and the big non-functional indentation change it caused, separately. Would you prefer (or tolerate ;) that style? Hmm, that might be nicer, I'm not 100% sure. I wouldn't say it's mandatory for a patch like this (and I'd say other peoples views on what's easy to review are at least as important as mine here). d ___ notmuch mailing list notmuch@notmuchmail.org http://notmuchmail.org/mailman/listinfo/notmuch