https://bugzilla.redhat.com/show_bug.cgi?id=1089770 --- Comment #2 from Michael Schwendt <bugs.michael@xxxxxxx> --- Not a full review, but one needs to start somewhere. Consider running "fedora-review -b 1089770" to point that tool at this ticket. It evaluates the "SRPM URL:" and "Spec URL:" lines and performs many helpful checks. > Summary: A LXC facility tool Even in the %summary there is enough room to expand "LXC". Ommitting the leading article improves readability. Summary: Linux Containers (LXC) facility tool https://fedoraproject.org/wiki/Examples_of_good_package_summaries > Source: http://prdownloads.sourceforge.net/lxcfacility/source/lxcf-%{version}.tar.gz https://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net > Prefix: %{_prefix} https://fedoraproject.org/wiki/Packaging:Guidelines#Relocatable_packages > Requires: python > Requires: python-IPy > Requires: virt-manager Please add comments on explicit dependencies. What exactly is needed from these packages? Files may move. And would these deps need to be arch-specific? > %build > make clean Comment on unusual command invocations like this one. Why is it needed? > %files > %defattr(-,root,root) https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions > %{_prefix}/sbin/lxcf > %{_libdir}/lxcf/lxcf-init Directory %_libdir/lxcf is not included yet. > %{_libdir}/lxcf/lxcf-keygen > %{_libdir}/lxcf/lxcf-rc > %{_libdir}/lxcf/lxcf-config > %{_libdir}/lxcf/lxcf-maintenance > [...] A comment in the spec file here would be beneficial, too. You list the specific file names of over 50 files. Is that really necessary? For example, do you want the build to fail if any of those files gets added/dropped/renamed in a future update? If so, a brief comment could explain that. Else, consider using '*' based wildcards to include files more conveniently. > %doc %attr(-,root,root) What does that line do? -- You are receiving this mail because: You are on the CC list for the bug. You are always notified about changes to this product and component _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review