I'd be happy to review this package. 
Look for a full review here in a few. 

OK - Package meets naming and packaging guidelines
OK - Spec file matches base package name.
OK - Spec has consistant macro usage.
OK - Meets Packaging Guidelines.
OK - License (GPLv2+)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
c5eda596bc3d142ff5cf6db7986e6459  postfix-logwatch-1.36.12.tgz
c5eda596bc3d142ff5cf6db7986e6459  postfix-logwatch-1.36.12.tgz.1
OK - BuildRequires correct
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Package has correct buildroot
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package has rm -rf RPM_BUILD_ROOT at top of %install

OK - Package compiles and builds on at least one arch.
OK - Package has no duplicate files in %files.
OK - Package doesn't own any directories other packages own.
OK - Package owns all the directories it creates.
See below - No rpmlint output.
OK - final provides and requires are sane:

SHOULD Items:

OK - Should build in mock.
OK - Should build on all supported archs
OK - Should function as described.
OK - Should have dist tag
OK - Should package latest version

Issues:

1. rpmlint says:

postfix-logwatch.noarch: E: non-executable-script
/etc/logwatch/scripts/services/postfix 0644

It's not clear to me if this does need to be executable, but if it was you
could run it standalone, so that might be better?

postfix-logwatch.noarch: W: conffile-without-noreplace-flag
/etc/logwatch/scripts/services/postfix

Should this have (noreplace) for upgrades to avoid messing up people's
customizations?

Hi Kevin.

I'm not sure how this has been packaged - postfix-logwatch runs either under
logwatch (wherein, it does not need to be executable), and standalone where it does.

In my new versions of the code, I have enabled perl's taint mode (via
#!/usr/bin/perl -T) for the standalone mode, and taint mode will not work under
logwatch.  As a standalone package, taint mode works fine.

I check postfix-logwatch into the logwatch cvs tree, so Fedora will
automatically get postfix-logwatch (named simply postfix in
/etc/logwatch/scripts/services/ directory) via logwatch updates.

I would recommend that this package named "postfix-logwatch" be installed as the
standalone program, which will a) be more up to date than that in logwatch, and
b) be decoupled from logwatch's schedule, which as a project is becoming
increasingly less active.

I have some significant new features I'm about to release - if you could hold
for a couple of days, I should be ready to release.

(In reply to comment #3)
> I'm not sure how this has been packaged - postfix-logwatch runs either under
> logwatch (wherein, it does not need to be executable), and standalone where it
does.

It is packaged to be used with logwatch.
 
> I check postfix-logwatch into the logwatch cvs tree, so Fedora will
> automatically get postfix-logwatch (named simply postfix in
> /etc/logwatch/scripts/services/ directory) via logwatch updates.

The main reason I built the package is to get this into Fedora EPEL, because the
postfix script in logwatch in RHEL does not produce such a nice output.
 
> I would recommend that this package named "postfix-logwatch" be installed as the
> standalone program, which will a) be more up to date than that in logwatch, and
> b) be decoupled from logwatch's schedule, which as a project is becoming
> increasingly less active.

These two benefits are no advantages of the standalone program, but only of this
package at all, imho. When the script is run from logwatch it is more up to date
and decoupled from logwatch's schedule, too.
 
> I have some significant new features I'm about to release - if you could hold
> for a couple of days, I should be ready to release.

This can be done.

I understand the motivation.  The version of logwatch has been historically
ancient in RHEL releases.

It may be not safe to assume that my current versions of postfix-logwatch will
just work in older versions of logwatch.  I'll make no guarantees there.  I've
also restructured the code into multiple perl modules, which will be shared with
amavis-logwatch, and future parsers.

One of the primary reasons I wrote postfix-logwatch, and added the standalone
feature, was so that the script could be run from the command line, allowing
more dynamic varieties of reports, which is not possible under logwatch without
modifying the conf/services/postfix.conf file before each run.  The standalone
config file is likely to be different than that used for logwatch (in
conf/services).  The newer --config_file option allows using varieties of
configuration files for specific reports, and command line options provide
further customization on the fly (esp. with new, upcoming features).

There is also a new man page; logwatch doesn't have module-specific man pages,
so this is a bit of a divergence.

As an "encouragement" to make the utility available in standalone mode, consider
speed and performance.  Logwatch is very clumsy and expensive in how it deals
with log files - it copies the primary log file, and when archives are being
searched, the archives as well, into a monolithic copy in the tmp directory. 
Performance on larger installations can be abysmal.  I encourage using
postfix-logwatch in standalone mode to avoid this excess.

I'd prefer not to deal with any confusions regarding which version filter is in
which version of logwatch.  If this can be minimized, I'm ok with it.

MrC

"may be not" => "may not be".

(as an addendum, an upcoming new option dynamically reconfigures and builds
several command line options on the fly, and adjusts how some config file
variables are interpreted.  This has more utility when running standalone).

I've updated postfix-logwatch with the new features indicated.  The release is
labeled 1.36.13pre5.

Whats the status here? 

Sorry for the delay.  Kevin, to whom was your question addressed?

I'll be promoting the devel release to released this week.  It will include a
couple minor bug fixes in the pre5 release, and the man page will be completed.

Till. Mike, Kevin.. what's the status here? I'd like to see this package in EPEL :)

I let version 1.36.12.1 run on my CentOS machines for several weeks and nothing
broke, therefore I would add the package as it is to EPEL.

This sounds excellent. Kevin, could you please continue the review ?

I'm ok with whatever choice is made here.  But consider that 1.36.12 is fairly
old, and these changes have been made since:

2008-01-14 (version: 1.36.13pre7)
 - New: Support ETRN rejects (option: RejectEtrn).
 - Fix: Include optional text in L1 output for header/body checks Hold
   messages.
 - Fix: Accept "unknown" as an IP in Connection rate limit messages.
   Thanks: Stefan Jakobs.
 - Fix: Improve MxError captures
 - Fix: Ignore "sql auxprop plugin ..." messages
 - Fix: Ignore more debug_peer_level=2 messages
 - Fix: Correct failure to ignore "connect to subsystem..." debug lines
 - Fix: Add missing RejectVerify to config file

2007-12-15 (version: 1.36.13pre6)
 - Fix: in BounceLocal and EnvelopeSenderDomains, set null domain
   and formatted host to '*unknown'.
 - Fix: Move postfix_warning before postfix_cleanup, as some cleanup
   warnings were caught as unmatched.
 - Fix: An Accepted message is now triggered by smtpd "client=..."
   and pickup "uid=..." log entries, instead of qmgr's "from=xxx,
   size=nnn, nrcpt=nnn" log lines.  A message may have been accepted
   during a previous time period, but delivery delays result in
   multiple qmgr delivery attempts, resulting in over counting
   Accepted messages.  Thanks: Stefan Jakobs
 - New: Bytes delivered is now broken down by Bytes sent via SMTP,
   LMTP, and forwarded, to be orthogonal with message counts.
 - Fix: Resent messages no longer reduce the number of messages
   accepted - this was a naive attempt at not counting, for example,
   messages released and requeued from a content filter's quarantine.

2007-11-14 (version: 1.36.13pre5)
 - New: Threshold limiting and Top N lists for every level in each
   Detail section.  Every level in each section can now be limited
   with minimum count thresholds and top N lists.  See the updated
   README file, the comments in the postfix-logwatch.conf file, and
   the new postfix-logwatch man page.  Requested by: Pavel Urban
 - New: Rejects can now be categorized by reject reply code.  A new
   option/variable "reject_reply_patterns" is a list of reject reply
   code regular expressions, which are used for categorizing rejects.
   This feature allows, for example, distinguishing 421 transmission
   channel closes from 45x errors. (eg. 450 mailbox unavailable, 451
   local processing errors, 452 insufficient storage).  The default
   list is: "5.. 4.. Warn" which creates three groups of rejects:
   permanent rejects, temporary failures, and reject warnings (as in
   warn_if_reject).  Requested by: Noel Jones
 - New: postfix-logwatch man page created (net yet complete)
 - New: Support for all access(5) actions.  See the "Level Limiter
   Options" section in the postfix-logwatch(1) man page, and "Common
   access control actions" in postfix-logwatch.conf.
 - New: Added envelope senders and envelope sender domains reports.
   These are disabled by default.  Enable with level limiter options
   --envelopesenders 1 and --envelopesenderdomains 1 (or 2 to
   also see senders listed under domains).  See also the
   postfix-logwatch.conf file.  Suggested by: Brendan
 - Change: Uncomment all variables in the config file.  This should
   help ensure the variables in the config file stay in sync with
   those used in the source.
 - Change: Merged sections SenderDelayNotification, DSNDelivered, and
   DSNUndelivered into NotificationSent, with sub-sections indicating
   the type of notification.  This gives the total number of sent
   notifications in the Summary section, and the breakdown by type
   of notification in the Detailed section.
 - Change: Removed "msgs" prefix from several options: msgsdeferred,
   msgsdelivered, msgsforwarded, msgsresent, msgssent, and msgssentlmtp
   are now deferred, delivered, forwarded, resent, sent and sentlmtp.
   The old options are still usable.
 - Change: force --help output to 80 chars.
 - Change: Taint mode is now on by default in standalone mode.  It is
   disabled upon installation in logwatch mode, as logwatch fails with
   taint mode enabled.
 - Change: Set the primary key in tlsserverconnect/tlsclientconnect
   options to type/cypher to reduce excessive level 2 output.
 - Change: Reported values that cannot be determined or are unavailable
   are prefixed with an asterisk (e.g. *unknown, *unspecified).
 - Change: Warn section title 'Warn action logged' changed simply to
   "Warned", to be consistent with other access/header_ and body_checks.
   The option is --warned, but --warn is still acceptable.
 - Fix: Significantly reduce memory footprint when detail < 5.
 - Fix: Usage and --help now correctly show only detail section
   level limiter options that are available.  Previously, summary-
   only counts were also display as level limiter options.
 - Fix: "too many errors after DATA" and "timeout after DATA" may 
   include a byte count, as in "(348 bytes)".  Thanks: John Beaver
 - Fix: Ignore postgrey 'delayed ...' lines
 - Fix: Allow logwatch --debug option to pass into postfix-logwatch
 - Fix: configuration file reading code was not properly warning on
   non-existent files
 - Fix: "filter" actions were incremented on "redirect" actions
 - Fix: Give more room to percentiles in delays percentiles table.
   A 5 day delay is 432000.000 seconds, and the previous table width
   was not sufficient.
 - Fix: --show_sect_vars command line option inadvertently required an
   argument; the name has been shortened to --sect_vars/--nosect_vars
   and correctly no longer requires an argument.  The longer names
   --[no]show_sect_vars work as well.  Thanks: Noel Jones
 - Fix: Handle some unmatched connect to failures in section
   ConnectToFailure.  Cleanup and consolidate several similar messages;
   add additional detail at level 3.
 - Fix: Increment postsuper Hold messages by the number of messages
   placed on hold.
 - Fix: Add "non-ESMTP response ..." messages to SmtpConversationError.
   First level is now the general SMTP error description.
 - Internal: Converted source to be package-based to allow code sharing
   with amavis-logwatch.  The single-file executable is auto-generated
   from the packages.
 - Internal: internal gen_test_log function to create sample log data
   for testing. Reads '#TD' comments from within postfix-logwatch.

2007-10-16 (version: 1.36.13pre4)
 - Fix: Handle messages "SSL_connect error to example.com: 0" and
   "Cannot start TLS: handshake failure" by coercing into warnings
   Thanks: Rob Sterenborg

2007-10-15 (version: 1.36.13pre3)
 - Fix: Handle "postmaster" DSNs
 - Fix: Handle "discarding EHLO keywords:" (ignored)
 - Internal: create bounce subroutine to handle bounce messages

2007-10-05 (version: 1.36.13pre2)
 - New: Initial support for postgrey (http://postgrey.schweikert.ch/)
   Requested by: Sebastian Wolfgarten

2007-10-05 (version: 1.36.13pre1)
 - New: Support for policy-spf software postfix-policyd-spf-perl
   Requested by: Nicodemo P. and Rob Sterenborg

I can bump the version to 1.36.13 final if necessary - its been operational for
quite some time.

Till: Would you like me to take another look at the last posted package here, or
would you like to spin up an updated one? 

Let me know and we can get this review moving... 

Any news here?

Please add a link to a site for the latest version of the filter inline; I tried getting the latest postfix file 
from http://www2.logwatch.org:81/ but it's incompatible with the logwatch in v5.1.

Hoping 5.1 gets a fix to stop barfing all over my amavisd and current-postfix log messages soon...

Chris: The latest versions are available at:

http://www.mikecappella.com/logwatch

Please try the devel version: 1.36.13pre7

I will have a pre8 version out this week, and if there are no issues, will
promote to 1.36.13.

I am sorry, but I do not have the time to continue this. I will reopen this bug
report, when I have, but maybe someone else wants to package this, which I do
not want to block.

I'm reopening this as I have been in discussion with Karel Klic (logwatch maintainer) about removing the (old) postfix script from the logwatch package, and distributing Mike's up-to-date script in a new postfix-logwatch package.

Mike, are you still there? Your website doesn't seem to work at the moment:

http://www.mikecappella.com/logwatch

No longer blocks FE-DEADREVIEW.

I'm here.  I've moved to sourceforge:

http://logreporters.sourceforge.net/
https://sourceforge.net/projects/logreporters/

Richard: were you going to submit a new updated package here for review?

I will get to it. Just haven't had much time lately to work on it.

Why don't you open a new review on the package when you are ready to submit? 

In the mean time we can close this one.