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

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: autofs - A tool for automatically mounting and unmounting filesystems


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=227631


orion at cora.nwra.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |NEEDINFO
               Flag|                            |fedora-review-,
                   |                            |needinfo?(ikent at redhat.com)




------- Additional Comments From orion at cora.nwra.com  2007-02-08 18:50 EST -------
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

    *




-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.