Bug 503336 - Review Request: newsbeuter - console based feed reader
Summary: Review Request: newsbeuter - console based feed reader
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Jan Klepek
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2009-05-31 13:05 UTC by Byron Clark
Modified: 2009-11-16 07:36 UTC (History)
7 users (show)

Fixed In Version: 2.0-8.fc12
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2009-11-16 07:36:27 UTC
Type: ---
Embargoed:
jan.klepek: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description Byron Clark 2009-05-31 13:05:02 UTC
Spec URL: http://theclarkfamily.name/fedora/newsbeuter.spec
SRPM URL: http://theclarkfamily.name/fedora/newsbeuter-2.0-3.fc11.src.rpm
Description: Configurable text-based feed reader.

Comment 1 Susi Lehtola 2009-06-06 18:34:10 UTC
- Listing locales explicitly is forbidden, you need to use %{find_lang}.
http://fedoraproject.org/wiki/Packaging/Guidelines#Handling_Locale_Files

- Explicitly requiring stfl, sqlite, libcurl, libxml2 is forbidden, the dependencies will be automatically picked up by rpm.
http://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires

- Is the prefix argument for both make and make install really necessary?

- I wouldn't include
 %{_defaultdocdir}/%{name}/*
instead I'd remove that directory after install and list the documentation under %doc.

Comment 2 Susi Lehtola 2009-06-06 18:35:43 UTC
(In reply to comment #1)
> - I wouldn't include
>  %{_defaultdocdir}/%{name}/*
> instead I'd remove that directory after install and list the documentation
> under %doc.  

In fact you have to fix this, since
 %doc README LICENSE
already creates %{_defaultdocdir}/%{name}-%{version} and puts the files there, having a %{_defaultdocdir}/%{name} too is confusing.

Comment 3 Byron Clark 2009-06-06 23:32:52 UTC
Unfortunately the prefix argument is required for both make and make install because it is used to build a #define for the location of the locale files.

All other comments have been fixed in the package.

Spec URL: http://theclarkfamily.name/fedora/newsbeuter.spec
SRPM URL: http://theclarkfamily.name/fedora/newsbeuter-2.0-4.fc11.src.rpm

Comment 4 Jan Klepek 2009-06-10 06:15:49 UTC
Summary:        The Mutt of Feed Readers

I don't think that this is good summary. 
Better would be imho "Configurable text-based feed reader", because it much more describe what is content of package. "The Mutt of Feed Readers" tell nothing to somebody who doesn't know mutt (if you refer to mail client) and for me it looks like commercial.

Comment 5 Byron Clark 2009-06-10 12:40:02 UTC
(In reply to comment #4)
> I don't think that this is good summary. 

I replaced the summary.

Spec URL: http://theclarkfamily.name/fedora/newsbeuter.spec
SRPM URL: http://theclarkfamily.name/fedora/newsbeuter-2.0-5.fc11.src.rpm

Comment 6 Jan Klepek 2009-06-21 21:48:05 UTC
when building:
Makefile:19: config.mk: No such file or directory

rpmlint is complaining
newsbeuter.i586: W: incoherent-version-in-changelog 0.20-5 ['2.0-5.fc11', '2.0-5']
newsbeuter.i586: W: spurious-executable-perm /usr/share/man/man1/podbeuter.1.gz
newsbeuter.i586: W: spurious-executable-perm /usr/share/man/man1/newsbeuter.1.gz

Comment 8 Thomas Janssen 2009-10-02 10:21:37 UTC
Ok, i take over that one, information is here: https://bugzilla.redhat.com/show_bug.cgi?id=502614
Some minor spec changes

Spec URL: http://thomasj.fedorapeople.org/reviews/newsbeuter.spec
SRPM URL: http://thomasj.fedorapeople.org/reviews/newsbeuter-2.0-7.fc10.src.rpm

[thomas@tusdell SPECS]$ rpmlint newsbeuter.spec ../SRPMS/newsbeuter-2.0-7.fc10.src.rpm ../RPMS/x86_64/newsbeuter-*
3 packages and 1 specfiles checked; 0 errors, 0 warnings.



-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

Comment 9 Thomas Janssen 2009-10-23 16:16:54 UTC
Ok, stfl (Newsbeuter dep) is in rawhide and i queued it for F-10 and F-11 already. Starting to work on that one now.

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

Comment 10 Mamoru TASAKA 2009-10-23 18:45:48 UTC
(Just for info that you don't have to remove the tags for blocker
 bugs even if they are closed)

Comment 11 Thomas Janssen 2009-10-27 21:52:32 UTC
Thanks Mamoru Tasaka.

Added CXXFLAGS=%{optflags}
Added Requires: stfl
Added ncurses-devel to BuildRequires

Spec URL: http://thomasj.fedorapeople.org/reviews/newsbeuter.spec
SRPM URL: http://thomasj.fedorapeople.org/reviews/newsbeuter-2.0-7.fc10.src.rpm

[thomas@tusdell ~]$ rpmlint rpmbuild/SPECS/newsbeuter.spec rpmbuild/SRPMS/newsbeuter-2.0-7.fc10.src.rpm rpmbuild/RPMS/x86_64/newsbeuter-*
3 packages and 1 specfiles checked; 0 errors, 0 warnings.

http://koji.fedoraproject.org/koji/taskinfo?taskID=1773771

Works great here.


-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

Comment 12 Jan Klepek 2009-11-11 19:11:55 UTC
1] ncurses-devel is still missing in BR

2] there is directory "test" which contains test suites, however it requires tuitest ruby package which is not yet in fedora.
what about performing tests in %check?

*license ok
*rpmlint ok
*package naming ok
*it is working
*license present
*package legible
*source url / md5sum ok
*build ok
*utf-8 ok

Will be approved when 1] is fixed.

Comment 13 Thomas Janssen 2009-11-11 21:07:52 UTC
Sorry, my bad. Looks like i did everything but not uploading the new Spec.

I did play around with %check and the lcov-run-all.sh, but it gave me a real hard time, failing on koji. Like: 
ruby: command not found
lcov: command not found
and so on. I removed it again. If it's not badly needed to run the tests, i would keep it out for now.

Spec URL: http://thomasj.fedorapeople.org/reviews/newsbeuter.spec
SRPM URL: http://thomasj.fedorapeople.org/reviews/newsbeuter-2.0-8.fc11.src.rpm

[thomas@tusdell ~]$ rpmlint rpmbuild/SPECS/newsbeuter.spec 
rpmbuild/SRPMS/newsbeuter-2.0-8.fc11.src.rpm 
rpmbuild/RPMS/x86_64/newsbeuter-*8*fc11*
3 packages and 1 specfiles checked; 0 errors, 0 warnings.

http://koji.fedoraproject.org/koji/taskinfo?taskID=1802301

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

Comment 14 Jan Klepek 2009-11-12 19:36:02 UTC
approved

Comment 15 Thomas Janssen 2009-11-12 20:58:39 UTC
Thanks for the review Jan Klepek.

New Package CVS Request
=======================
Package Name: newsbeuter
Short Description: Configurable text-based feed reader
Owners: thomasj
Branches: F-11 F-12
InitialCC: 

-- 
Fedora Bugzappers volunteer triage team
https://fedoraproject.org/wiki/BugZappers

Comment 16 Jason Tibbitts 2009-11-13 19:33:21 UTC
CVS done.

Comment 17 Fedora Update System 2009-11-14 19:19:26 UTC
newsbeuter-2.0-8.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/newsbeuter-2.0-8.fc12

Comment 18 Fedora Update System 2009-11-16 07:36:22 UTC
newsbeuter-2.0-8.fc12 has been pushed to the Fedora 12 stable repository.  If problems still persist, please make note of it in this bug report.


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