Bug 204694 - Review Request: zvbi - Raw VBI, Teletext and Closed Caption decoding library
Review Request: zvbi - Raw VBI, Teletext and Closed Caption decoding library
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Rex Dieter
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT 204421
  Show dependency treegraph
 
Reported: 2006-08-30 17:09 EDT by Ian Chapman
Modified: 2011-02-06 17:39 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-07 20:47:23 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Ian Chapman 2006-08-30 17:09:08 EDT
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 02:13:50 EDT
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 08:14:01 EDT
> 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 08:28:25 EDT
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 08:39:07 EDT
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 12:37:52 EDT
> 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 15:40:54 EDT
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 15:20:23 EDT
Excellent, looks good, APPROVED.
Comment 8 Ian Chapman 2006-09-07 20:47:23 EDT
Thanks for the review.
Comment 9 Dmitry Butskoy 2011-02-04 10:12:28 EST
Package Change Request
======================
Package Name: zvbi
New Branches: el5 el6
Owners: buc
Comment 10 Kevin Fenzi 2011-02-06 17:39:30 EST
Git done (by process-git-requests).

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