fedora-review denied: [Bug 227631] Review Request: autofs - A tool for automatically mounting and unmounting filesystems

[bugzilla at redhat.com]

Bug 227631: Review Request: autofs - A tool for automatically mounting and
unmounting filesystems
Product: Fedora Extras
Version: devel
Component: Package Review

Orion Poplawski <orion at cora.nwra.com> has denied Orion Poplawski
<orion at cora.nwra.com>'s request for fedora-review:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=227631

------- Additional Comments from Orion Poplawski <orion at cora.nwra.com>
Some of these are minor, but does bring about a nice consistency to the spec
files.

- rpmlint checks return:

W: autofs no-url-tag

Is there any?

W: autofs unversioned-explicit-obsoletes autofs-ldap
The specfile contains an unversioned Obsoletes: token, which will match all
older, equal and newer versions of the obsoleted thing.  This may cause update
problems, restrict future package/provides naming, and may match something it
was originally not inteded to match -- make the Obsoletes versioned if
possible.

W: autofs macro-in-%changelog _xxx
Macros are expanded in %changelog too, which can in unfortunate cases lead
to the package not building at all, or other subtle unexpected conditions that
affect the build.  Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally
odd entries eg. in source rpms, which is rarely wanted.  Avoid use of macros
in %changelog altogether, or use two '%'s to escape them, like '%%foo'.

W: autofs mixed-use-of-spaces-and-tabs (spaces: line 481, tab: line 102)
The specfile mixes use of spaces and tabs for indentation, which is a
cosmetic annoyance.  Use either spaces or tabs for indentation, not both.

- Why define %version and %release?  Aren't they are defined automatically?

- Consider Requires: chkconfig rather than Requires: /sbin/chkconfig, easier on

the depsolvers and unlikely to change.	Similar for /bin/bash, others.

- %clean should just be:
rm -rf $RPM_BUILD_ROOT


- exit 0 in %preun seems unneeded.

- %defattr should be:
%defattr(-,root,root,-)

- There's an empty %doc line.

- Still need to ship %dir /misc?  You don't with /net.

- This:

%dir %{_libdir}/autofs
%{_libdir}/autofs/*

could be:

%{_libdir}/autofs/

Good:

- package meets naming guidelines
- package meets packaging guidelines
- license ( ) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86)
- 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

    *