Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 204694 - Review Request: zvbi - Raw VBI, Teletext and Closed Caption decoding library
Summary: Review Request: zvbi - Raw VBI, Teletext and Closed Caption decoding library
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Rex Dieter
QA Contact: Fedora Package Reviews List
URL:
Whiteboard:
Depends On:
Blocks: FE-ACCEPT 204421
TreeView+ depends on / blocked
 
Reported: 2006-08-30 21:09 UTC by Ian Chapman
Modified: 2011-02-06 22:39 UTC (History)
3 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2006-09-08 00:47:23 UTC
Type: ---
Embargoed:
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Ian Chapman 2006-08-30 21:09:08 UTC
Spec URL: http://dribble.org.uk/reviews/zvbi.spec
SRPM URL: http://dribble.org.uk/reviews/zvbi-0.2.22-1.src.rpm
Description: 

ZVBI provides functions to capture and decode VBI data. The vertical blanking
interval (VBI) is an interval in a television signal that temporarily suspends
transmission of the signal for the electron gun to move back up to the first
line of the television screen to trace the next screen field. The vertical
blanking interval can be used to carry data, since anything sent during the VBI
would naturally not be displayed; various test signals, closed captioning, and
other digital data can be sent during this time period.

Comment 1 Parag AN(पराग) 2006-09-01 06:13:50 UTC
rpmlint on binary rpm reported
zvbi-0.2.22-1.fc6.i386.rpm 
I: zvbi checking
W: zvbi no-reload-entry /etc/rc.d/init.d/zvbid
In your init script (/etc/rc.d/init.d/your_file), you don't
have a 'reload' entry, which is necessary for good functionality.

W: zvbi incoherent-init-script-name zvbid
The init script name should be the same as the package name in lower case.

all above warnings have their descriptions given about how to solve them so
follow that.



Comment 2 Rex Dieter 2006-09-01 12:14:01 UTC
> you don't have a 'reload' entry, which is necessary for good functionality.

And if the daemon doesn't support reload?  (alias it to restart?)

> The init script name should be the same as the package name in lower case

Not required, hence rpmlint marking this as a warning only, not an error.  IMO,
the script name should be the same as the daemon in question.

Comment 3 Rex Dieter 2006-09-01 12:28:25 UTC
I can review this.  At first glance, package looks clean, just a few items off
the top of my head so far:

1.  in -devel: Change
Requires: zvbi = %{version}-%{release}
to the less error-prone:
Requires: %{name} = %{version}-%{release}

2. in %post/%postun fonts, change
/usr/bin/fc-cache %{_datadir}/fonts
to
/usr/bin/fc-cache %{_datadir}/fonts/%{name}
no need to tell fc-cache to reparse *all* of %_datadir/fonts, when we're only
interested in %_datadir/fonts/%name

3. -fonts: I'm pretty sure there's no real need for
Requires:           fontconfig
Requires(post):     /usr/bin/fc-cache
Requires(postun):   /usr/bin/fc-cache
The pkg doesn't *really* need/use fontconfig, and the calls to fc-cache in
scriptlets are wrapped with:
if [ -x /usr/bin/fc-cache ]; then
...
fi

Comment 4 Rex Dieter 2006-09-01 12:39:07 UTC
Ammend item 2:
Only should change %post, I'd recommend:
/usr/bin/fc-cache -f  %{_datadir}/fonts/%{name}
shouldn't change %postun, since %{_datadir}/fonts/%{name} no longer exists

Comment 5 Ian Chapman 2006-09-01 16:37:52 UTC
> W: zvbi no-reload-entry /etc/rc.d/init.d/zvbid
> In your init script (/etc/rc.d/init.d/your_file), you don't
> have a 'reload' entry, which is necessary for good functionality.
> 
> W: zvbi incoherent-init-script-name zvbid
> The init script name should be the same as the package name in lower case.
> 
> all above warnings have their descriptions given about how to solve them so
> follow that.

I'm in agreement with Rex on this one, I think calling the init script zvbi in 
this case doesn't make much sense, particularly when the daemon is really a 
small subset of the whole zvbi package, as opposed to being the primary 
function. It would require more than a simple namechange as the init script 
would need to be patched so that the 'subsystems' had matching names. Overkill 
I think for simply dropping the trailing 'd'. 

With wrt the reload option as it's considered optional I would rather not add 
it. IIRC primary reason for a reload option is to tell the daemon to reload its 
config files without quitting and starting again which is very useful for 
daemons that don't instantly stop or start such as squid, but in this case 
zvbid doesn't load configs.


Comment 6 Ian Chapman 2006-09-01 19:40:54 UTC
Here's the latest version incorporating the three fixes from above.

Spec URL: http://dribble.org.uk/reviews/zvbi.spec
SRPM URL: http://dribble.org.uk/reviews/zvbi-0.2.22-2.src.rpm

Comment 7 Rex Dieter 2006-09-07 19:20:23 UTC
Excellent, looks good, APPROVED.

Comment 8 Ian Chapman 2006-09-08 00:47:23 UTC
Thanks for the review.

Comment 9 Dmitry Butskoy 2011-02-04 15:12:28 UTC
Package Change Request
======================
Package Name: zvbi
New Branches: el5 el6
Owners: buc

Comment 10 Kevin Fenzi 2011-02-06 22:39:30 UTC
Git done (by process-git-requests).


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