Bug 503336 - Review Request: newsbeuter - console based feed reader
Review Request: newsbeuter - console based feed reader
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
low Severity medium
: ---
: ---
Assigned To: Jan Klepek
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2009-05-31 09:05 EDT by Byron Clark
Modified: 2009-11-16 02:36 EST (History)
7 users (show)

See Also:
Fixed In Version: 2.0-8.fc12
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-11-16 02:36:27 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
jan.klepek: fedora‑review+
tibbs: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Byron Clark 2009-05-31 09:05:02 EDT
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 14:34:10 EDT
- 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 14:35:43 EDT
(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 19:32:52 EDT
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 02:15:49 EDT
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 08:40:02 EDT
(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 17:48:05 EDT
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 06:21:37 EDT
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 12:16:54 EDT
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 14:45:48 EDT
(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 17:52:32 EDT
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 14:11:55 EST
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 16:07:52 EST
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 14:36:02 EST
approved
Comment 15 Thomas Janssen 2009-11-12 15:58:39 EST
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 14:33:21 EST
CVS done.
Comment 17 Fedora Update System 2009-11-14 14:19:26 EST
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 02:36:22 EST
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.