Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=657403 Hans de Goede <hdegoede@xxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- Flag| |fedora-review? --- Comment #6 from Hans de Goede <hdegoede@xxxxxxxxxx> 2011-01-05 05:54:17 EST --- Full review done: Good: - rpmlint checks return: spice-gtk-python.x86_64: W: no-documentation spice-gtk-tools.x86_64: W: no-documentation spice-gtk-tools.x86_64: W: no-manual-page-for-binary snappy spice-gtk-tools.x86_64: W: no-manual-page-for-binary spicy 6 packages and 0 specfiles checked; 0 errors, 4 warnings. These can all be ignored. - package meets naming guidelines - package meets packaging guidelines - license (LGPLv2+) 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 - devel package ok - no .la files - post/postun ldconfig ok - devel requires base package n-v-r Some minor nitpicks / should fix items: - The Group tag for the main package should be: System Environment/Libraries - "Requires: %{name} = %{version}" should be dropped - The %description for the main package talks about a gtk client and libraries, but the gtk client is part of the tools package - Likewise the %description for tools does not mention the gtk client - The "Requires: %{name} = %{version}" does not specify the release in the package, when subpackages depend on other packages in the same SRPM the requires should specify the full NEVR (name epoch version release) like the Requires in the devel sub package - The prefered form for the defattr is: %defattr(-,root,root,-) rather then: %defattr(-, root, root) - There is no need to specify dir owner ship and the files inside it if you want the package to own the dir and all files, for example this: %dir %{_includedir}/spice-client-glib/ %{_includedir}/spice-client-glib/*.h Can be written simply as: %{_includedir}/spice-client-glib Likewise for spice-client-gtk, also you could consider using wildcards, condensing the %files devel to: %{_libdir}/libspice-client-g*.so %{_includedir}/spice-client-g* %{_libdir}/pkgconfig/spice-client-g*.pc %{_datadir}/gir-1.0/SpiceClientG*-1.0.gir %doc %{_datadir}/gtk-doc/html/* -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review