Bug 616983 - Review Request: yarssr - Yet Another RSS Reader is an RSS reader for GNOME notification area
Summary: Review Request: yarssr - Yet Another RSS Reader is an RSS reader for GNOME no...
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
Depends On:
TreeView+ depends on / blocked
Reported: 2010-07-21 21:06 UTC by Fabian Grutschus
Modified: 2010-10-03 19:40 UTC (History)
4 users (show)

Clone Of:
Last Closed: 2010-10-03 19:40:06 UTC

Attachments (Terms of Use)

Description Fabian Grutschus 2010-07-21 21:06:32 UTC
Spec URL: http://rpm.lubyte.de/yarssr/yarssr.spec
SRPM URL: http://rpm.lubyte.de/yarssr/RPMS/noarch/yarssr-0.2.2-1.fc13.noarch.rpm
Description: "Yet Another RSS Reader is an RSS aggregator and reader that displays its results in the GNOME  notification area. To view the contents of the feed just click the menu-item and it will launch in your favorite browser. It is written in Perl and uses gtk2-perl for it's interface."

Yarssr is a fine small tool for RSS feeds and there is no other program covers that in Fedora.

> yarssr.i386: W: spelling-error %description -l en_US aggregator -> aggregation, aggregated, aggregate
copy-paste from the website
> yarssr.i386: W: only-non-binary-in-usr-lib
ok, only Perl scripts
> yarssr.i386: W: no-manual-page-for-binary yarssr
yarssr doesn't provide man pages

It's my first package. Requesting a Sponsor.

Comment 1 Xavier Bachelot 2010-07-21 23:16:01 UTC
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.
- BuildRoot is not mandatory anymore, but can still be useful if you want to have EPEL branches.
  Same for the %clean section.
  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.

Comment 2 Fabian Grutschus 2010-07-22 02:19:57 UTC
Updated the spec File under http://rpm.lubyte.de/yarssr/yarssr.spec
Sources can be found here: http://rpm.lubyte.de/yarssr/SOURCES/
the new package at: http://rpm.lubyte.de/yarssr/RPMS/noarch/yarssr-0.2.2-1.fc13.noarch.rpm

rmplint output:
[crash@homebox ~]$ rpmlint rpmbuild/RPMS/noarch/yarssr-0.2.2-1.fc13.noarch.rpm 
yarssr.noarch: W: spelling-error %description -l en_US aggregator -> aggregation, aggregated, aggregate
yarssr.noarch: W: invalid-license GPL
yarssr.noarch: W: only-non-binary-in-usr-lib
yarssr.noarch: W: no-manual-page-for-binary yarssr
1 packages and 0 specfiles checked; 0 errors, 4 warnings

Comment 3 Matthias Runge 2010-07-22 09:14:54 UTC
Hey, just some small hints:

- please try rpmlint -i. This gives you some really useful hints. 
- Are you really sure, your rpm is a SRPM?
- when updateing the spec, you should increase the release
- are you sure, this is really supported? Last version dates from 2005! It looks to me like a dead prject.

Comment 4 Xavier Bachelot 2010-07-22 09:51:55 UTC
More comments :

- License is actually GPL+, sorry for my earlier misleading comment.

- Use the most compressed source, here .tar.bz2 rather than.tar.gz 

- Use parallel make when building.

- Missing BuildRequires: gettext.

- Missing BuildRequires: desktop-file-utils.

- According to desktop-file-validate, the desktop file has a few issues :
yarssr.desktop: warning: key "Encoding" in group "Desktop Entry" is deprecated
yarssr.desktop: warning: value "Application;Network;" for key "Categories" in group "Desktop Entry" contains a deprecated value "Application"
yarssr.desktop: error: value "text/html" for string list key "MimeType" in group "Desktop Entry" does not have a semicolon (';') as trailing character

- The package must not own %{_datadir}/applications, only the desktop file.

- %defattr is not correct.

- The specfile mixes use of spaces and tabs for indentation, which is a cosmetic annoyance. Use either spaces or tabs for indentation, not both.

- When submitting a package for review, you need to provide the spec file and the source rpm, not the binary rpm.

- You must add a changelog entry and bump the release tag accordingly every time you submit a modified version of your package.

Comment 5 Mamoru TASAKA 2010-08-21 15:32:58 UTC
What is the status of this bug?

Comment 6 Mamoru TASAKA 2010-09-01 17:46:42 UTC
Again ping?

Comment 7 Mamoru TASAKA 2010-09-11 16:12:11 UTC
ping again??

Comment 8 Mamoru TASAKA 2010-09-22 17:11:15 UTC
I will close this bug as NOTABUG if no response from the reporter
is received within ONE WEEK.

Comment 9 Mamoru TASAKA 2010-10-03 19:40:06 UTC
Once closing.

If someone wants to import this package into Fedora, please
open a new review request and mark this bug as a duplicate of
the new one.

Thank you!

Note You need to log in before you can comment on or make changes to this bug.