Bug 616983

Summary: Review Request: yarssr - Yet Another RSS Reader is an RSS reader for GNOME notification area
Product: [Fedora] Fedora Reporter: Fabian Grutschus <f.grutschus>
Component: Package ReviewAssignee: Nobody's working on this, feel free to take it <nobody>
Status: CLOSED NOTABUG QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: low    
Version: rawhideCC: fedora-package-review, mrunge, notting, xavier
Target Milestone: ---   
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2010-10-03 15:40:06 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Bug Depends On:    
Bug Blocks: 201449    

Description Fabian Grutschus 2010-07-21 17:06:32 EDT
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 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.
Comment 2 Fabian Grutschus 2010-07-21 22:19:57 EDT
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 05:14:54 EDT
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 05:51:55 EDT
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 11:32:58 EDT
What is the status of this bug?
Comment 6 Mamoru TASAKA 2010-09-01 13:46:42 EDT
Again ping?
Comment 7 Mamoru TASAKA 2010-09-11 12:12:11 EDT
ping again??
Comment 8 Mamoru TASAKA 2010-09-22 13:11:15 EDT
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 15:40:06 EDT
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!