https://bugzilla.redhat.com/show_bug.cgi?id=1089770 --- Comment #5 from NIWA Hideyuki <niwa.hideyuki@xxxxxxxxxxxxxx> --- Hi, thank you for the comment. I corrected them as follows. - > 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 > I thought that your description was good. Please let me use it for the summary as it is. Summary: Linux Containers (LXC) facility tool > > > Source: http://prdownloads.sourceforge.net/lxcfacility/source/lxcf-%{version}.tar.gz > > https://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net > I corrected it as follows. Source0: http://downloads.sourceforge.net/lxcfacility/source/%{name}-%{version}.tar.gz > > > Prefix: %{_prefix} > > https://fedoraproject.org/wiki/Packaging:Guidelines#Relocatable_packages > Prefix was erased. > > > 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? > These "Require" was erased. > > > > %build > > make clean > > Comment on unusual command invocations like this one. Why is it needed? > I erased it by thinking it was unnecessary. > > > %files > > %defattr(-,root,root) > > https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions > I erased this. > > %{_prefix}/sbin/lxcf > > %{_libdir}/lxcf/lxcf-init > I corrected it as follows. %{_sbindir}/lxcf %{_libdir}/lxcf/ > 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. > It is as you say. I corrected it as follows. %{_libdir}/lxcf/ > > > %doc %attr(-,root,root) > > What does that line do? I erased it because I was unnecessary. Thanks. -- 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