Bug 204421

Summary: Review Request: kdetv - KDE application for watching TV
Product: [Fedora] Fedora Reporter: Ian Chapman <packages>
Component: Package ReviewAssignee: Rex Dieter <rdieter>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Package Reviews List <fedora-package-review>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: dennis
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: 2006-09-10 17:13:33 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 204694    
Bug Blocks: 163779    

Description Ian Chapman 2006-08-28 23:43:22 UTC
Spec URL: http://dribble.org.uk/reviews/kdetv.spec
SRPM URL: http://dribble.org.uk/reviews/kdetv-0.8.9-1.src.rpm
Description: 

kdetv is a KDE application that allows you to watch television on your
GNU/Linux box running KDE. You probably know it as QtVision, the completely
rewritten version of KWinTV.

Note:
rpmlint warnings about dangling symlinks can be ignored, as they point to files in another package. I also think the warnings about devel files in non devel package can also be ignored because KDE often seems to requite the .so and .la files (although I may be wrong :) )

Comment 1 Ian Chapman 2006-08-30 21:22:19 UTC
Updated version with support for zvbi

http://dribble.org.uk/reviews/kdetv.spec
http://dribble.org.uk/reviews/kdetv-0.8.9-2.src.rpm

Note: zvbi is also waiting for review :)

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=204694

Comment 2 Rex Dieter 2006-09-01 14:29:31 UTC
Looks good, off-hand a few quickies:

0.  Curious, why is this GPL/LPGL?

1.  SHOULD: instead of manually deleting applnk, instead use 
desktop-file-install --delete-original 

2.  SHOULD: Since this isn't a .desktop provided by fedora, imo, you needn't use:
desktop-file-install --vendor="fedora"
but instead use --vendor="" 


Comment 3 Ian Chapman 2006-09-01 20:40:25 UTC
(In reply to comment #2)

> 0.  Curious, why is this GPL/LPGL?

Included in the original archive are the two license files COPYING (GPL) and 
COPYING.LIB (LGPL) so I thought, perhaps incorrectly that kdetv's libs were 
LGPL but kdetv is itself GPL. :-)

 
> 1.  SHOULD: instead of manually deleting applnk, instead use 
> desktop-file-install --delete-original 

Thanks, I've fixed this.


> 2.  SHOULD: Since this isn't a .desktop provided by fedora, imo, you needn't 
use:
> desktop-file-install --vendor="fedora"
> but instead use --vendor="" 
 
I've fixed this also, the latest version should be listed below, thanks :)

http://dribble.org.uk/reviews/kdetv.spec
http://dribble.org.uk/reviews/kdetv-0.8.9-3.src.rpm

Comment 4 Rex Dieter 2006-09-08 17:12:26 UTC
Built fine in mock, rpmlint is mostly harmless:

$rpmlint kdetv-0.8.9-3.fc6.i386.rpm
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_tomsmocomp.la
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_chromakill.la
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_v4l.la
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_xmlchannels.la
W: kdetv dangling-relative-symlink /usr/share/doc/HTML/sv/kdetv/common ../common
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_mirror.la
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_overscan.la
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_colourinversion.la
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_xv.la
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_simon.la
W: kdetv dangling-relative-symlink /usr/share/doc/HTML/nl/kdetv/common ../common
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_bilinear.la
E: kdetv script-without-shellbang /usr/lib/libkvideoio.la
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_haze.la
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_libzvbidecoder.la
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_elegant.la
E: kdetv script-without-shellbang /usr/lib/libkdetv.la
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_kwintvchannels.la
W: kdetv dangling-relative-symlink /usr/share/doc/HTML/et/kdetv/common ../common
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_greedyh.la
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_v4l2.la
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_xawtvchannels.la
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_none.la
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_zappingchannels.la
W: kdetv devel-file-in-non-devel-package /usr/lib/libkdetvvideo.so
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_linearblend.la
W: kdetv dangling-relative-symlink /usr/share/doc/HTML/da/kdetv/common ../common
E: kdetv script-without-shellbang /usr/lib/libkdetvvideo.la
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_oss.la
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_greedy2frame.la
W: kdetv dangling-relative-symlink /usr/share/doc/HTML/fr/kdetv/common ../common
W: kdetv devel-file-in-non-devel-package /usr/lib/libkdetv.so
W: kdetv dangling-relative-symlink /usr/share/doc/HTML/pt/kdetv/common ../common
W: kdetv dangling-relative-symlink /usr/share/doc/HTML/en/kdetv/common ../common
W: kdetv dangling-relative-symlink /usr/share/doc/HTML/ru/kdetv/common ../common
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_sharpness.la
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_telex.la
W: kdetv dangling-relative-symlink /usr/share/doc/HTML/it/kdetv/common ../common
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_greedy.la
W: kdetv devel-file-in-non-devel-package /usr/lib/libkvideoio.so
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_channelsuite.la
E: kdetv script-without-shellbang /usr/lib/kde3/kdetv_csvchannels.la

You can omit:
BuildRequires:  libXext-devel
since that's already pulled in by qt-devel.

Else, looks good, APPROVED.

Comment 5 Ian Chapman 2006-09-09 19:10:20 UTC
Thanks for the review, I'll drop that BR before import.