Bug 951812

Summary: Review Request: canto-curses - Client portion of canto RSS reader.
Product: [Fedora] Fedora Reporter: James Walsh <walshy86>
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: unspecified    
Version: rawhideCC: bugs.michael, i, msuchy, package-review, sanjay.ankur, walshy86
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: 2016-02-08 09:05:24 EST Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On: 951811    
Bug Blocks: 201449    

Description James Walsh 2013-04-13 06:44:40 EDT
Spec URL: http://walshy007.studioteabag.com/fedora/canto-curses.spec
SRPM URL: http://walshy007.studioteabag.com/fedora/canto-curses-0.8.4-1.fc17.src.rpm
Description: canto-curses client. Interfaces with canto-daemon to provide quite a lovely and lightweight rss reader.
Fedora Account System Username:walshy007
Comment 1 Michael Schwendt 2013-06-19 05:14:09 EDT
* Since package "canto" has been included in the distribution and has been retired just recently, the following process applies:

  http://fedoraproject.org/wiki/Orphaned_package_that_need_new_maintainers#Claiming_Ownership_of_a_Deprecated_Package


* The split into canto-daemon and canto-curses requires the Package Renaming Process to be followed:

  http://fedoraproject.org/wiki/Package_Renaming_Process


* Try to do a self-review of your package(s) with the help of the following page:
  https://fedoraproject.org/wiki/Packaging:ReviewGuidelines

  [ https://fedoraproject.org/wiki/Join_the_package_collection_maintainers ]
  [ https://fedoraproject.org/wiki/Category:Package_Maintainers ]


* Any idea whether the existing bug reports still apply or what to do with them?
  https://admin.fedoraproject.org/pkgdb/acls/bugs/canto


* Now on to the spec file, which could be simplified and polished a bit:


> Summary: Canto RSS curses client

Repeating the name of the software is a waste of space in most cases, because it duplicates information included in the package name and/or description. And new users would not know what "Canto" is or why it would be relevant. The setup.py file contains an alternative summary, which is clear and concise, IMO. One could drop the "Next-gen" marketing speak to make it even more short and to the point:

  Summary: Next-gen console RSS/Atom reader
  Summary: Console RSS/Atom reader


> Version: 0.8.4
> Source: http://codezen.org/static/canto-curses-0.8.4.tar.gz

You are free to use %{version} in the Source tag and in other places of the spec file. That can be convenient when updating the spec file for a new version.


> #version numbers required
> %global canto_ver 0.8.4 
> %global py3_ver %(echo `%{__python3} -c "import sys;
> sys.stdout.write(sys.version[:3])"`)

Really? %version would be 0.8.4 already, too, and there's only one place in the spec file where you use  %py3_ver:

  %{python3_sitearch}/Canto_curses-%{canto_ver}-py%{py3_ver}.egg-info

It wouldn't be a bad idea to simply use the '*' wildcard character here to include any .egg-info file in that directory:

  %{python3_sitearch}/Canto_curses*.egg-info
or
  %{python3_sitearch}/Canto_curses-*-py*.egg-info


> Requires: python3,canto-daemon,ncurses

"python3" and "ncurses" should be dropped here.

The dependency on "ncurses" is automatic already (due to libncursesw being linked) and arch-specific, too.
https://fedoraproject.org/wiki/Packaging:Guidelines#Explicit_Requires

There is an automatic dependency on 'python(abi) = 3.3' and libpython, too.
https://fedoraproject.org/wiki/Packaging:Python


* In the build.log:

> -I/usr/local/include -I/opt/local/include 

Not an immediate problem for clean buildroots, such as with Mock or Fedora koji, but unnecessarily altering the search path for headers is a regular problem for users, who want to (re)build distribution packages and forget that they have installed own stuff in those paths, too.

No need to change/fix it, I just mention it here.


> %files
> %defattr(644,root,root,-)

%defattr is not needed anymore since RPM 4.4, so it can be dropped for any of the currently active distribution releases:
https://fedoraproject.org/wiki/Packaging:Guidelines#File_Permissions

> %attr(755,root,root) %{_bindir}/canto-curses

If you only want to mark it executable, prefer chmod +x during %install, and figure out whether upstream's installation script could mark them +x already. Executables are such a common case, so that using %attr is too much, because preferably it is used only for non-standard user/group ownership and special permissions.


> %{_mandir}/man1/canto-curses.1.gz

%{_mandir}/man1/canto-curses.1*  would be cleaner, since it allows for a changed/customised/dropped compression technique.


> %changelog
> *Fri Apr 12 2013 - walshy86@gmail.com - 0.8.4-1

https://fedoraproject.org/wiki/Packaging:Guidelines#Changelogs
Comment 2 James Walsh 2013-06-21 04:00:59 EDT
Thanks for the feedback, I will adjust the spec file, retest and post back later this evening after reading the items you've linked.
Comment 3 Miroslav Suchý 2015-07-21 09:52:55 EDT
Any progress here? Are you still interrested in this package?
Comment 4 Miroslav Suchý 2016-02-08 09:05:24 EST
No response. Closing as dead review. If you ever want to continue, please resubmit.