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(a)bachelot.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
CC| |xavier(a)bachelot.org
--- Comment #1 from Xavier Bachelot <xavier(a)bachelot.org> 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.