Note: This is a beta release of Red Hat Bugzilla 5.0. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Also email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback here.
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...
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Nobody's working on this, feel free to take it
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: FE-DEADREVIEW
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)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
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.

rpmlint:
> 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.
  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.

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.
http://fedoraproject.org/wiki/Licensing

- Use the most compressed source, here .tar.bz2 rather than.tar.gz 
https://fedoraproject.org/wiki/Packaging:SourceURL

- Use parallel make when building.
https://fedoraproject.org/wiki/Packaging:Guidelines#Parallel_make

- Missing BuildRequires: gettext.
https://fedoraproject.org/wiki/Packaging:Guidelines#Handling_Locale_Files

- Missing BuildRequires: desktop-file-utils.
https://fedoraproject.org/wiki/Packaging:Guidelines#desktop-file-install_usage

- 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.
https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions

- 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.
http://fedoraproject.org/wiki/Package_Review_Process#Contributor

- 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.