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 1438853
| Summary: | Review Request: lldpd - an ISC-licensed implementation of LLDP | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | James Hogarth <james.hogarth> |
| Component: | Package Review | Assignee: | Dan Horák <dan> |
| Status: | CLOSED CURRENTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | dan, package-review, rc040203 |
| Target Milestone: | --- | Flags: | dan:
fedora-review+
|
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | If docs needed, set a value | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2017-05-11 20:56:01 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: | |||
|
Description
James Hogarth
2017-04-04 14:46:44 UTC
Koji scratch builds: rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=18782014 f26: https://koji.fedoraproject.org/koji/taskinfo?taskID=18782015 f25: https://koji.fedoraproject.org/koji/taskinfo?taskID=18782023 epel7: https://koji.fedoraproject.org/koji/taskinfo?taskID=18782022 Not a formal review just some remarks: - Building package is not verbose. Please append --disable-silent-rules to %configure. - The package contains a tarball named "0.9.7.tar.gz" I would propose to change Source0 into: Source0: https://github.com/%{gh_owner}/%{name}/archive/%{version}.tar.gz#/%{name}-%{version}.tar.gz This lets downloading from github create a tarball named "lldpd-0.9.7.tar.gz" formal review is here, see the notes explaining OK* and BAD statuses below:
OK source files match upstream:
f3ed143ba16fb4232a237f51cddaee276af896c8 0.9.7.tar.gz
OK package meets naming and versioning guidelines.
OK specfile is properly named, is cleanly written and uses macros consistently.
OK dist tag is present.
OK license field matches the actual license.
OK license is open source-compatible (ISC). License text included in package.
OK latest version is being packaged.
OK* BuildRequires are proper.
OK compiler flags are appropriate.
OK package builds in mock (Rawhide/x86_64).
OK debuginfo package looks complete.
BAD rpmlint is silent.
OK final provides and requires look sane.
N/A %check is present and all tests pass.
OK shared libraries are added to the regular linker search paths with ldconfig call.
OK owns the directories it creates.
OK doesn't own any directories it shouldn't.
OK no duplicates in %files.
OK file permissions are appropriate.
OK correct scriptlets present.
OK code, not content.
OK documentation is small, so no -docs subpackage is necessary.
OK %docs are not necessary for the proper functioning of the package.
OK headers in devel subpackage
OK pkgconfig files in devel subpackage
OK no libtool .la droppings.
OK not a GUI app.
- please do the 2 changes Ralf proposed, use %{name}-%{version}.tar.gz style for the source and use a verbose build (easier to read the build log than inspect *.o files for used compiler flags)
- doxygen is present in BR, but there are no docs generated
- rpmlint complains a bit
lldpd.x86_64: W: summary-not-capitalized C lldpd is an ISC-licensed implementation of LLDP
lldpd.x86_64: W: name-repeated-in-summary C lldpd
lldpd.src: W: summary-not-capitalized C lldpd is an ISC-licensed implementation of LLDP
lldpd.src: W: name-repeated-in-summary C lldpd
lldpd-devel.x86_64: W: summary-not-capitalized C lldpd is an ISC-licensed implementation of LLDP
-> Summary: ISC-licensed implementation of LLDP
lldpd.x86_64: W: non-standard-uid /var/lib/lldpd lldpd
lldpd.x86_64: W: non-standard-gid /var/lib/lldpd lldpd
lldpd-devel.x86_64: W: no-documentation
lldpd-devel.x86_64: W: only-non-binary-in-usr-lib
lldpd.x86_64: W: hidden-file-or-dir /usr/lib/.build-id
lldpd.x86_64: W: hidden-file-or-dir /usr/lib/.build-id
lldpd.x86_64: W: conffile-without-noreplace-flag /etc/lldpd.d/README.conf
-> OK, can be ignored
lldpd.x86_64: E: zero-length /usr/share/doc/lldpd/ChangeLog
-> drop the file from %doc, needs git tree and NEWS is sufficient, Contribute.md can dropped too
lldpd.src:9: W: mixed-use-of-spaces-and-tabs (spaces: line 8, tab: line 9)
lldpd.x86_64: E: missing-call-to-chdir-with-chroot /usr/sbin/lldpd
lldpd.x86_64: W: shared-lib-calls-exit /usr/lib64/liblldpctl.so.4.8.0 exit.5
lldpd.x86_64: W: non-ghost-in-run /run/lldpd/chroot
lldpd.x86_64: W: non-ghost-in-run /run/lldpd
lldpd-devel.x86_64: W: no-dependency-on lldpd/lldpd-libs/liblldpd
-> need fix or comment
Thanks for the feedback, both of you. * Source0 line updated * Summary changed to suggestion * configure option updated * requires added to subpackage on main package * looking at more detail it wanted not just doxygen but the pdf part as well to produce pdf docs... that feels silly so I've pulled the BR Koji builds: F25: https://koji.fedoraproject.org/koji/taskinfo?taskID=18796508 F26: https://koji.fedoraproject.org/koji/taskinfo?taskID=18796713 RAW: https://koji.fedoraproject.org/koji/taskinfo?taskID=18796875 EL7: https://koji.fedoraproject.org/koji/taskinfo?taskID=18797292 Spec URL: https://fedorapeople.org/~jhogarth/lldpd/lldpd.spec SRPM URL: https://fedorapeople.org/~jhogarth/lldpd/lldpd-0.9.7-2.fc27.src.rpm Looks good now, package is APPROVED. Thanks Dan ... and good news is it only required a minor maintainer patch to get it built against EPEL6 as well :) Letting the developer know the changes I made in the patch to see if he's willing to upstream. Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/lldpd lldpd-0.9.7-4.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-676f5655d9 lldpd-0.9.7-4.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2017-cedacd0aaf lldpd-0.9.7-4.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-8741526fc3 lldpd-0.9.7-5.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-77fac90af3 lldpd-0.9.7-5.fc24 has been submitted as an update to Fedora 24. https://bodhi.fedoraproject.org/updates/FEDORA-2017-8363c4e8ec lldpd-0.9.7-5.el6 has been submitted as an update to Fedora EPEL 6. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-6ab500b6b5 lldpd-0.9.7-5.fc26 has been submitted as an update to Fedora 26. https://bodhi.fedoraproject.org/updates/FEDORA-2017-7356de7a31 lldpd-0.9.7-5.el7 has been submitted as an update to Fedora EPEL 7. https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-fe76c20831 lldpd-0.9.7-5.el6 has been pushed to the Fedora EPEL 6 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-6ab500b6b5 lldpd-0.9.7-5.el7 has been pushed to the Fedora EPEL 7 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2017-fe76c20831 lldpd-0.9.7-5.fc24 has been pushed to the Fedora 24 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-8363c4e8ec lldpd-0.9.7-5.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-77fac90af3 lldpd-0.9.7-5.fc26 has been pushed to the Fedora 26 testing repository. If problems still persist, please make note of it in this bug report. See https://fedoraproject.org/wiki/QA:Updates_Testing for instructions on how to install test updates. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-7356de7a31 lldpd-0.9.7-5.fc26 has been pushed to the Fedora 26 stable repository. If problems still persist, please make note of it in this bug report. lldpd-0.9.7-5.fc24 has been pushed to the Fedora 24 stable repository. If problems still persist, please make note of it in this bug report. lldpd-0.9.7-5.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report. lldpd-0.9.7-5.el6 has been pushed to the Fedora EPEL 6 stable repository. If problems still persist, please make note of it in this bug report. lldpd-0.9.7-5.el7 has been pushed to the Fedora EPEL 7 stable repository. If problems still persist, please make note of it in this bug report. |