Bug 568641 - Review request: lldpad - Daemon and configuration tool for Intel LLDP Agent
Summary: Review request: lldpad - Daemon and configuration tool for Intel LLDP Agent
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
Target Milestone: ---
Assignee: Jan Vcelak
QA Contact: Fedora Extras Quality Assurance
Depends On:
TreeView+ depends on / blocked
Reported: 2010-02-26 08:27 UTC by Jan Zeleny
Modified: 2013-03-04 01:27 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2010-04-20 11:00:06 UTC
jvcelak: fedora-review+
tibbs: fedora-cvs+

Attachments (Terms of Use)

Description Jan Zeleny 2010-02-26 08:27:16 UTC
SPEC: http://jzeleny.fedorapeople.org/packages/lldpad/lldpad.spec
SRPM: http://jzeleny.fedorapeople.org/packages/lldpad/lldpad-0.9.26-1.fc12.src.rpm

This is substitude for original dcbd package, it has the same functionality plus it adds lldp protocol support.

Comment 1 Jan Vcelak 2010-02-26 10:14:48 UTC
[ ] rpmlint

$ rpmlint -i ./*.rpm
lldpad.src:9: W: mixed-use-of-spaces-and-tabs (spaces: line 1, tab: line 9)
The specfile mixes use of spaces and tabs for indentation, which is a cosmetic
annoyance.  Use either spaces or tabs for indentation, not both.

lldpad.x86_64: E: incoherent-subsys /etc/rc.d/init.d/lldpad dcbd
The filename of your lock file in /var/lock/subsys/ is incoherent with your
actual init script name. For example, if your script name is httpd, you have
to use 'httpd' as the filename in your subsys directory. It is also possible
that rpmlint gets this wrong, especially if the init script contains
nontrivial shell variables and/or assignments.  These cases usually manifest
themselves when rpmlint reports that the subsys name starts a with '$'; in
these cases a warning instead of an error is reported and you should check the
script manually.

4 packages and 0 specfiles checked; 1 errors, 1 warnings.

[*] meets Naming Guidelines
[*] meets Licensing Guilelines
[*] sources match upstream version
[?] all patches have upstream bug link or comment

No comments at all (although the meaning is quite obvious from their names).

[*] license specified in specfile, license file included
[*] specfile in American English
[*] specfile is legible
[*] valid BuildRoot
[*] buildroot cleanup before %install
[ ] uses macros consitently

Please, use either %{xxx} style or $XXX style macros. Pick a style and use it consistently throughout your packaging.
I suggest replacing $RPM_BUILD_ROOT with %{buildroot}

Example from your specfile: mkdir -p $RPM_BUILD_ROOT%{_initddir}

[*] pkgconfig files in -devel subpackage
[*] no static libraries included
[*] no staticly linked executables
[*] scriptlets requirements splitted
[*] %preun, %postun scriptlets running only in certain situations
[*] scriptlets are sane
[*] package is relocatable
[*] all files and directories included
[*] permissions and ownership specified
[*] all filenames valid UTF-8
[*] contains code, or permissible content
[*] documentation doesn't need separate package
[*] %doc does not affect runtime
[*] subpackages with fully versioned dependency
[*] builds in koji

Package rename requirements:

[*] Provides is correct
[*] Obsoletes is correct

Comment 3 Jan Vcelak 2010-02-26 11:13:19 UTC
Thank you. ACCEPT.

Comment 4 Jan Vcelak 2010-02-26 12:22:52 UTC
Just to be clear: I'm aware of the fact that this is a re-review.

Comment 5 Jan Zeleny 2010-02-26 12:47:45 UTC
New Package CVS Request
Package Name: lldpad
Short Description: This package contains the Linux user space daemon and
configuration tool for Intel LLDP Agent with Enhanced Ethernet support for the
Data Center.
Owners: jzeleny
Branches: F-12

Comment 6 Jason Tibbitts 2010-02-26 17:57:46 UTC
If your "short description" wraps onto three lines, it's not really "short".

I'll use "Daemon and configuration tool for Intel LLDP Agent".  Of course, it would be really nice if your %description actually included some mention of what an LLDP agent actually is.  This is the kind of thing that's supposed to be noticed as part of the package review.

Comment 7 Jason Tibbitts 2010-02-26 18:00:32 UTC
CVS done (by process-cvs-requests.py).

I also added an F-13 branch as that seems to have been missed.

Comment 8 Jan Zeleny 2010-04-20 11:00:06 UTC
Package added, closing this review request.

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