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=616983 Xavier Bachelot <xavier@xxxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |xavier@xxxxxxxxxxxx --- Comment #1 from Xavier Bachelot <xavier@xxxxxxxxxxxx> 2010-07-21 19:16:01 EDT --- I'm not a sponsor, so I can't officially review this package, but here are a couple comments that I hope will help clean up the spec a bit. - All Requires on one line is not legible, please put one per line. - It's better to use Requires on virtual perl Provides (eg. perl(XML::RSS) rather than perl-XML-RSS) - Most of the explicit perl Requires can probably be automatically detected, please check after building and remove unneeded ones accordingly. - Align tags values, it will be more legible imho. - According to comments in the script, License is GPL, not GPLv2+. - Replace Source with Source0. - Source URL is not the canonical format for SourceForge URLs. https://fedoraproject.org/wiki/Packaging:SourceURL#Sourceforge.net - BuildRoot is not mandatory anymore, but can still be useful if you want to have EPEL branches. Same for the %clean section. https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag On the same topic, buildroot needs to be cleaned at the start of the %install section in EPEL : rm -rf $RPM_BUILD_ROOT Choose one case or the other and make the spec consistent. - BuildArchitectures is usually written as BuildArch. - Tag order is currently a bit messy, a better order could be : Name, Version, Release, Summary, Group, License, URL, Source, BuildRoot, BuildArch - PREFIX=/usr should be PREFIX=%{_prefix} - RPM_OPT_FLAGS is unneeded, this is a noarch package - Don't generate the desktop file in the spec, add it as Source1. - Use macros in the %files section : - /usr/bin --> %{_bindir} - /usr/share --> %{_datadir} - /usr/lib --> %{_libdir} - Replace /usr/share/yarssr/* with %{_datadir}/yarss or even %{_datadir}/%{name}, all files below the dir will be owned w/o the need of the wildcard. - /usr/share/locale/en/LC_MESSAGES/yarssr.mo should already be taken care of by %find_lang, remove it from the %file section. -- 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