OK, there are a few things that you need to change in this package:

* Please don't override localstatedir. This is set properly in the Fedora config.

* In the Summary, please describe the package without using its name. Tell me
what it does, not what it is called. :) Also, please be sure to capitalize the
first word (and don't end it with a period).

* Release should not be wholly macro driven, this is something that you want to
increment by hand. I strongly suggest that you consider using the Dist Tag
system for this, see: http://fedoraproject.org/wiki/Packaging/DistTag

* We have a specific, explicit system for identifying the License tag. Your
package should have "License: GPLv2+", see:
http://fedoraproject.org/wiki/Packaging/LicensingGuidelines

* Please use the %{name} and %{version} macros everywhere that you have "libpst"
and "0.6.7" hardcoded (except in Name: and Version:, of course). This ensures a
much easier update path, with less work on your part. For example, use them in
the Source: field, in your mkdir and mv commands, and in the %files entries.

* Please use one of the approved BuildRoot settings, which are listed here:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-b4fdd45fa76cbf54c885ef0836361319ab962473

* Do not set Vendor or Packager in the spec file. The Fedora buildsystem will
set these values automatically.

* Do not use AutoReqProv. This is one of the big benefits of rpm! We want those
provides!

* In your %description, please don't have any line longer than 80 characters.
This ensures that it displays cleanly on a terminal screen. You can have
multiple lines. :)

* Please use %setup -q instead of just %setup. This quiets the output from the
tarball unpacking, which helps us save on logfile size in the buildsystem.

* In %build, please replace your long configure invocation with %configure,
which is a macro that evaluates to the same thing (and more).

* In %build, please use make %{_?smp_mfiles} instead of just make. See:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-525c7d76890cb22df33b759c65c35c82bf434d2e

* In %install, there is no reason to check the buildroot before deleting it.
Since you've set it in the spec file, it will always be safe to simply delete it
as the first step in the %install process.

* In %install, replace your long make install invocation with simply:
make DESTDIR=$RPM_BUILD_ROOT install
It will achieve the same result, but will avoid embedding the buildroot into the
installed files (and is also much smaller).

* In %install, you do not need to make the docdir and manually copy the files
into it. rpm will do this for you, for any files marked in %files as doc. 
To do this, just get rid of the mkdir and mv commands from %install, and delete
these lines from %files:
%doc %{_mandir}/*
%docdir %{_datadir}/doc/libpst-0.6.7
%{_datadir}/doc/libpst-0.6.7
Then, add this line to %files:
%doc AUTHORS COPYING ChangeLog NEWS README
That's it! Same end result, so much less pain. :)

* You do not need to have empty %pre, %post, %preun, and %postun entries. Just
take them out entirely.

* In your %clean entry, please add: rm -rf $RPM_BUILD_ROOT

* In %files, please use %defattr(-,root,root,-) instead of %defattr(-,root,root)

* In %files, please don't use %doc %{_mandir}/*. There are two problems with
this line:
1. %{_mandir} is automagically marked as %doc when it is used in %files, you do
not need to specify this.
2. %{_mandir}/* will cause this package to own all of the created directories
under %{_mandir}, which in the case of your package are "man1/" and "man5/". You
only want to own the manpages installed in those directories, so replace the
line with:
%{_mandir}/man1/*
%{_mandir}/man5/*

* In the %changelog, please ensure the following:
Everytime that you make any change, no matter how small or trivial, add a
changelog entry, in a supported format. The supported formats are described
here:
http://fedoraproject.org/wiki/Packaging/Guidelines#head-b7d622f4bb245300199c6a33128acce5fb453213

* The naming of this package "libpst" is a little strange, given that it doesn't
actually have any libraries in it. A better name might be "pst-utils". Normally,
I wouldn't say anything, but since I know that you are the upstream, I figured I
would throw it out there. If you like the "libpst" name, that's fine, you can
keep using it here.

I highly recommend that you read through:
http://fedoraproject.org/wiki/Packaging/Guidelines and
http://fedoraproject.org/wiki/Packaging/NamingGuidelines. Please fix the spec
file to reflect these changes, and I will be happy to do a review, and sponsor
you upon success. :)

Thanks! 

What is the policy on the change log in the .spec file versus the gnu
ChangeLog/NEWS files? Are we supposed to duplicate every entry from ChangeLog
into the .spec file, or is the changelog in the spec file only for packaging
related changes?

Yes, the name is strange, but that is the original name of the project, and I
did not want to arbitrarily change it. At some point we may be able to
standardize the interface enough to create a libpst.so used by the actual
utilities, and also usable by other projects. That will further add to the
naming confusion.

The changelog in the .spec file is for changes to the spec file itself, not for
copying the packaged source "ChangeLog". :)

Delay caused by work on 0.6.8 with another conversion utility. This now requires
ImageMagick and gd.

The libpst.spec <http://www.five-ten-sg.com/libpst/packages/libpst.spec> is
built via ./configure (which we need to run to get 'make distcheck' anyway) from
libpst.spec.in <http://www.five-ten-sg.com/libpst/packages/libpst.spec.in>. 

The packaging guidelines say "Every time you make changes, that is, whenever you
increment the E-V-R of a package, add a changelog entry.". The version is
contained in the autoconf configure.in, so when we bump that and rebuild, we
automatically get a new libpst.spec with a new version number. But since we
don't generally update the libpst.spec.in, that results in no change log entry
in libpst.spec. Do you want us to manually add changelog entries in
libpst.spec.in for every version bump in configure.in, even though the packaging
is not really changing? I can see advantages to both yes and no answers.

%files
%doc AUTHORS COPYING ChangeLog NEWS README

That caused problems, since the %doc seems to remove the documentation
directory, which has already been populated by autoconf with html help files. My
solution is to have autoconf also handle those GNU standard files, and then just
reference 

%files
%docdir %{_datadir}/doc/%{name}-%{version}
%{_datadir}/doc/%{name}-%{version}

with the documentation content fully populated by autoconf make install.

Sorry for the delay in responding. I'm not an advocate of .spec.in files for the
reasons that you describe. In Fedora, your spec file is maintained independently
of your source tree, so you do need to add changelog entries in that file.

As to the %docdir, as long as those files are in that directory, it is fine.

Ok, I will change the workflow scripts here to ensure that the changelog entries
in .spec.in (and therefore in .spec) are consistent with the ones in the gnu
NEWS and ChangeLog files.

Now that F9 is out, what is the next step here? Updated tarballs and spec file
at <http://www.five-ten-sg.com/libpst/packages/>.

Good:

- rpmlint checks return:
libpst.src: W: strange-permission libpst.spec 0600
libpst.x86_64: W: incoherent-version-in-changelog 0.6.9 0.6.9-1.fc10

I'm ok with ignoring the permission on the spec file, but you should really fix
the changelog versioning. Just remember to put the version-release at the end
(right now, you're just putting version).

- package meets naming guidelines
- package meets packaging guidelines
- license (GPLv2+) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream (6321e94601697a20b9850a3c4eec718b77869ec8)
- package compiles on devel (x86_64)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file 

This is approved. Thanks for your patience! :)

You should be able to pick up the process around here:
https://fedoraproject.org/wiki/PackageMaintainers/Join#Watch_for_Feedback

I will sponsor you.

Since release is 1.fc10, what do you want for changelog entries that are
distribution independent? Or do we need to maintain separate .spec files for
fc10 and rhel5 (for example).

Looking at the above URL, does that procedure require a fedora client system? I
only have RHEL/Centos systems here.

Shouldn't require a fedora client system.

As to the release in the changelog, just use "1" without the disttag. This will
be correct for all distributions.

Just checking in, I haven't see you apply for cvsextras in the Fedora Account
System...

Sorry, I got way to busy this week. This is on my list for this weekend.

New Package CVS Request
=======================
Package Name: libpst
Short Description: Utilities to convert Outlook .pst files to other formats
Owners: carllibpst
Branches: F-10
InitialCC:
Cvsextras Commits: yes

cvs done.

./common/cvs-import.sh /tmp/libpst-0.6.14-1.src.rpm
cd devel; make build

That worked, but trying to import into the F-10 branch does not:

./common/cvs-import.sh -b F-10 /tmp/libpst-0.6.14-1.src.rpm
Checking out module: 'libpst'
ERROR: "libpst/F-10" does not exist!

Is that supposed to work, or is there some other requirement?

There is no F-10 branch because there is no F-10 release. devel is a "magic"
branch because it is always there, but right now, it is what will become F-10.

F-8 and F-9 are the other active branches right now.

Built in rawhide, closing this review ticket out.