On May 9, 2010, at 11:42 PM, Robert Spier wrote:
Assuming
http://github.com/msimerson/qpsmtpd/commit/6948c27205ebdec045604bd8d6bf6a3a04c35a8a
is the output of this experiment...
I'm not convinced that "one big" tidy is actually worth the
hassle/mess.
Do we peel off the bandaid slowly, or just rip it off? I've been on
the mailing list a very, very short time and I've already seen two
issues that perltidy uniformity would have prevented.
If we were to just fix the "big issues", I think that would help most
places without causing the huge churn, and hard to review change.
By "big issues" I mean:
tab vs spaces
trailing whitespace
I was going to include 4 vs 2 in there, but I'm not sure even that
makes a huge difference.
If we're going to do this, we need to get all outstanding patches
merged and just bite the bullet. It'll only suck during the
transition. (And for anyone backporting patches.)
Which is worse, one large hard to review change, or a hundred smaller
ones? I'm of the opinion that whitespace changes should not be
combined with other changes, as they make the 'significant' changes
harder to identify.
And for those backporting patches, they can skip the perltidy commit
and run perltidy on their own repo.
But with the exception of some annoying tab/space stuff, what's the
big win from doing this?
It avoids the death from a thousand paper cuts syndrome. I believe
it's ultimately less painful to perltidy the entire source tree all at
once (with no other code changes) than to perltidy one file at a time,
as other code changes are being made, making future commits larger and
with more changes than necessary.
The use of consistent formatting makes it easier for new authors to
contribute to qpsmtpd. The presence of a .perltidyrc file in the
distro communicates a set of expectations to a developer. When said
author roams around the source code and finds that the .perltidyrc
rules are merely suggestions, it communicates an entirely new set of
expectations.
Should all future check ins be expected to conform to the perltidy
rules? If not, the .perltidy file should be removed entirely. Or
splattered with comments that its presence is merely a suggestion, not
an expectation.
Matt
Matt Simerson wrote:
There's a nifty .perltidyrc file in the git repo.
There's also a lot of code that uses tab indents, 2 space indents,
and 4 space indents. And of course, code that uses a mixture of the
three, with local variations thrown in for good measure.
I figure that the least invasive time to do a wholesale perltidy is
right after a release, when most of the outstanding patches and
forks that will get merged in already have been. And a whitespace
change should not be combined with other changes. So I did this on
my fork:
find . -name '*.pm' -exec perltidy -b {} \;
find plugins -type f -exec perltidy -b {} \;
perltidy -b qpsmtpd*
find . -name '*.bak' -delete
And then manually perused through the changes, found the instances
where perltidy did not do The Right Thing[TM] (like altering the
contents of a here doc), ran the test suite, and submitted the
change for inclusion. Ask asked me to post the change to this list
for comment.
Will this change disrupt anyone significantly? Is there anything I
can do that makes this change less disruptive (ie, avoid files in
list .... ).
Thanks,
Matt
Begin forwarded message:
From: mailer-dae...@lists-nntp.develooper.com
Date: May 3, 2010 11:32:14 AM PDT
To: m...@tnpi.net
Hi. This is the qmail-send program at lists-nntp.develooper.com.
I'm afraid I wasn't able to deliver your message to the following
addresses.
This is a permanent error; I've given up. Sorry it didn't work out.
<perlmail-qpsm...@onion.perl.org>:
ezmlm-reject: fatal: Sorry, I don't accept messages larger than
400000 bytes (#5.2.3)
--- Below this line is a copy of the message.
<snip>
Is the .perltidy file included for a reason?
---
lib/Apache/Qpsmtpd.pm | 71 +-
lib/Danga/Client.pm | 88 ++-
lib/Danga/TimeoutSocket.pm | 16 +-
lib/Qpsmtpd.pm | 919 +++++++++++
+----------
lib/Qpsmtpd/Address.pm | 116 ++--
lib/Qpsmtpd/Auth.pm | 115 ++--
lib/Qpsmtpd/Command.pm | 41 +-
lib/Qpsmtpd/ConfigServer.pm | 178 +++--
lib/Qpsmtpd/Connection.pm | 138 ++--
lib/Qpsmtpd/Constants.pm | 74 +-
lib/Qpsmtpd/DSN.pm | 232 +++---
lib/Qpsmtpd/Plugin.pm | 199 +++---
lib/Qpsmtpd/PollServer.pm | 224 +++---
lib/Qpsmtpd/Postfix.pm | 261 ++++---
lib/Qpsmtpd/Postfix/Constants.pm | 129 ++--
lib/Qpsmtpd/SMTP.pm | 1226 ++++++++++++++
+--------------
lib/Qpsmtpd/SMTP/Prefork.pm | 37 +-
lib/Qpsmtpd/TcpServer.pm | 233 +++---
lib/Qpsmtpd/TcpServer/Prefork.pm | 89 ++-
lib/Qpsmtpd/Transaction.pm | 293 ++++----
lib/Qpsmtpd/Utils.pm | 1 -
plugins/async/check_earlytalker | 120 ++--
plugins/async/dns_whitelist_soft | 2 +-
plugins/async/queue/smtp-forward | 142 ++--
plugins/async/require_resolvable_fromhost | 138 ++--
plugins/async/rhsbl | 2 +-
plugins/async/uribl | 41 +-
plugins/auth/auth_checkpassword | 39 +-
plugins/auth/auth_cvm_unix_local | 56 +-
plugins/auth/auth_flat_file | 30 +-
plugins/auth/auth_ldap_bind | 393 +++++-----
plugins/auth/auth_vpopmail | 61 +-
plugins/auth/auth_vpopmail_sql | 71 +-
plugins/auth/authdeny | 6 +-
plugins/check_badmailfrom | 61 +-
plugins/check_badmailfromto | 69 +-
plugins/check_badrcptto | 28 +-
plugins/check_badrcptto_patterns | 26 +-
plugins/check_basicheaders | 43 +-
plugins/check_earlytalker | 178 +++--
plugins/check_loop | 32 +-
plugins/check_norelay | 36 +-
plugins/check_relay | 116 ++--
plugins/check_spamhelo | 20 +-
plugins/content_log | 24 +-
plugins/count_unrecognized_commands | 61 +-
plugins/dns_whitelist_soft | 169 ++--
plugins/dnsbl | 331 +++++----
plugins/domainkeys | 111 ++--
plugins/dont_require_anglebrackets | 12 +-
plugins/greylisting | 302 ++++----
plugins/help | 48 +-
plugins/hosts_allow | 28 +-
plugins/http_config | 30 +-
plugins/ident/geoip | 14 +-
plugins/ident/p0f | 144 ++--
plugins/logging/adaptive | 77 +-
plugins/logging/connection_id | 63 +-
plugins/logging/devnull | 2 +-
plugins/logging/file | 85 ++-
plugins/logging/syslog | 33 +-
plugins/logging/transaction_id | 58 +-
plugins/logging/warn | 66 +-
plugins/milter | 170 +++--
plugins/noop_counter | 32 +-
plugins/parse_addr_withhelo | 24 +-
plugins/queue/exim-bsmtp | 28 +-
plugins/queue/maildir | 203 +++---
plugins/queue/postfix-queue | 51 +-
plugins/queue/qmail-queue | 172 +++--
plugins/queue/smtp-forward | 80 +-
plugins/quit_fortune | 20 +-
plugins/random_error | 42 +-
plugins/rcpt_ok | 56 +-
plugins/rcpt_regexp | 1 +
plugins/relay_only | 12 +-
plugins/require_resolvable_fromhost | 237 +++---
plugins/rhsbl | 242 ++++---
plugins/sender_permitted_from | 118 ++--
plugins/spamassassin | 311 ++++----
plugins/tls | 150 ++--
plugins/tls_cert | 99 ++--
plugins/uribl | 275 ++++---
plugins/virus/aveclient | 187 +++--
plugins/virus/bitdefender | 36 +-
plugins/virus/clamav | 208 +++---
plugins/virus/clamdscan | 97 ++--
plugins/virus/hbedv | 206 +++---
plugins/virus/kavscanner | 238 +++---
plugins/virus/klez_filter | 46 +-
plugins/virus/sophie | 56 +-
plugins/virus/uvscan | 170 +++--
qpsmtpd | 4 +-
qpsmtpd-async | 250 ++++---
qpsmtpd-forkserver | 488 ++++++------
qpsmtpd-prefork | 178 +++--
t/Test/Qpsmtpd.pm | 67 +-
t/Test/Qpsmtpd/Plugin.pm | 7 +-
98 files changed, 6724 insertions(+), 5885 deletions(-)
<snip>
