https://bugzilla.redhat.com/show_bug.cgi?id=991531
--- Comment #5 from Michael Schwendt <[email protected]> --- > Requires: sysstat,perl(Text::BarGraph) If you specified such requirements on separate lines Requires: sysstat Requires: perl(Text::BarGraph) you could add comments more easily. Interesting here would be to comment on what exactly from "sysstat" is needed. That's just a package name, and that package contains multiple tools. If the Perl script executes individual tools from sysstat, it would be a sign of packaging quality to mention that. And if you fear that an executable might move from one package to another, you could depend on its full path even (with no metadata lookup penalty, because file paths are stored in primary metadata). > Requires: perl(Text::BarGraph) When it's strictly needed at run-time, it's clever to also require it at build-time as some sort of existance-check: BuildRequires: perl(Text::BarGraph) > %build > %prep > %setup -q It's okay to have an empty %build section. It would even be okay to have no such section at all (if rpmlint didn't warn about it). Better yet is to keep these sections sorted in the order they are executed during build, i.e. %prep -> %build -> %install [-> %check] > mkdir -p %{buildroot}%{_bindir} > chmod a+x %{name} > cp -p %{name} %{buildroot}%{_bindir} From the Hints Department, the "install" tool is very popular among packagers, because it can replace those three mkdir/chmod/cp lines: install -D -p -m0755 %{name} %{buildroot}%{_bindir}/%{name} > %changelog Not so important during review, but practising %changelog entry maintenance would be good: https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=WGNGrAFcZK&a=cc_unsubscribe _______________________________________________ package-review mailing list [email protected] https://admin.fedoraproject.org/mailman/listinfo/package-review
