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@xxxxxxxxxxxxx changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |NEEDINFO Flag| |fedora-review-, | |needinfo?(ikent@xxxxxxxxxx) ------- Additional Comments From orion@xxxxxxxxxxxxx 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review