Bug 227631: Review Request: autofs - A tool for automatically mounting and unmounting filesystems Product: Fedora Extras Version: devel Component: Package Review Orion Poplawski <orion@xxxxxxxxxxxxx> has denied Orion Poplawski <orion@xxxxxxxxxxxxx>'s request for fedora-review: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=227631 ------- Additional Comments from Orion Poplawski <orion@xxxxxxxxxxxxx> 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 * _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review