[Bug 461338] Review Request: dahdi - Userspace tools to configure the DAHDI kernel modules

bugzilla at redhat.com bugzilla at redhat.com
Thu Oct 9 09:13:05 UTC 2008


Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


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





--- Comment #3 from Peter Robinson <pbrobinson at gmail.com>  2008-10-09 05:13:04 EDT ---
A few small bits that need to be cleaned up.

+ rpmlint output

rpmlint -i dahdi-2.0.0-0.4.fc9.src.rpm
1 packages and 0 specfiles checked; 0 errors, 0 warnings.
rpmlint -i dahdi*2.0.0-0.4.fc9.x86_64.rpm
dahdi.x86_64: E: no-status-entry /etc/rc.d/init.d/dahdi
In your init script (/etc/rc.d/init.d/your_file), you don't have a 'status'
entry, which is necessary for good functionality.
5 packages and 0 specfiles checked; 1 errors, 0 warnings.

+ package name satisfies the packaging naming guidelines
+ specfile name matches the package base name
+ package should satisfy packaging guidelines
+ license meets guidelines and is acceptable to Fedora
+ license matches the actual package license

+ %doc includes license file
  you don't need to include README etc in all subpackages, just the main
package
+ spec file written in American English
+ spec file is legible

+ upstream sources match sources in the srpm

c09f880e67305bf5561c8030958b9fb9  dahdi-linux-2.0.0.tar.gz
31e48ed37e43662b0f8fbf146e192b66  dahdi-tools-2.0.0.tar.gz

+ package successfully builds on at least one architecture
  tested with a koji scratch build on all platforms
- BuildRequires list all build dependencies
  the perl BuildReq needs to be moved up with the rest, the BuildReq are for
entire package group, not sub packages
n/a %find_lang instead of %{_datadir}/locale/*
+ binary RPM with shared library files must call ldconfig in %post and %postun
+ does not use Prefix: /usr
+ package owns all directories it creates
+ no duplicate files in %files
+ %defattr line
+ %clean contains rm -rf $RPM_BUILD_ROOT
+ consistent use of macros
+ package must contain code or permissible content
n/a large documentation files should go in -doc subpackage
+ files marked %doc should not affect package
+ header files should be in -devel
n/a static libraries should be in -static
n/a packages containing pkgconfig (.pc) files need 'Requires: pkgconfig'
+ libfoo.so must go in -devel
+ devel must require the fully versioned base
+ packages should not contain libtool .la files
- packages containing GUI apps must include %{name}.desktop file
  see gtk2 point below.
+ packages must not own files or directories owned by other packages
+ %install must start with rm -rf %{buildroot} etc.
+ filenames must be valid UTF-8

Optional:

n/a if there is no license file, packager should query upstream
n/a translations of description and summary for non-English languages, if
available
+ reviewer should build the package in mock
+ the package should build into binary RPMs on all supported architectures
+ review should test the package functions as described (basic testing)
+ scriptlets should be sane
n/a pkgconfig files should go in -devel
+ shouldn't have file dependencies outside /etc /bin /sbin /usr/bin or
/usr/sbin

A couple of other things:

- Why are the modprobe noreplace? If there are new devices or blacklists added
between releases this wouldn't be automatically picked up.
%config(noreplace) %{_sysconfdir}/modprobe.d/dahdi
%config(noreplace) %{_sysconfdir}/modprobe.d/dahdi.blacklist
- the main dahdi package requires the perl modules so should depend on it. Are
the perl modules usable without the main package installed, if not there is no
point of the subpackage
- why does it need gtk2-devel? It builds fine without it and the configure
script doesn't check for it.
- The release number should be 2.0.0-1 not 2.0.0-0.4 (ie the final 0.x usually
designates a pre or RC release)

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.




More information about the Fedora-package-review mailing list