Bug 668197
| Summary: | Review Request: ledctl - LED control app for Intel(R) storage controllers | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Jiri Moskovcak <jmoskovc> |
| Component: | Package Review | Assignee: | Ondrej Vasik <ovasik> |
| Status: | CLOSED CURRENTRELEASE | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | dfediuck, fedora-package-review, notting, ovasik |
| Target Milestone: | --- | Flags: | ovasik:
fedora-review+
j: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | Bug Fix | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2011-02-16 09:46:40 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
Jiri Moskovcak
2011-01-08 21:48:48 UTC
I see no SCM request to process here. formal review is here, see the notes explaining OK* and BAD statuses below:
N/A source files match upstream:
can't be checked, as there is no public upstream url for sources download
it's quite common in the case of intel packages in early life.
Just for reference, sha1sums of checked components:
$ sha256sum ledctl.spec
01ef25293aaf5dd83f499a86f8c8d83ae6816eaf5f7a0f6884d15f14b2669dcd ledctl.spec
$ sha256sum ledctl-0.1.tgz
209a5430ffb166de65e9949f196e05be8da4937150997f1e509e27bd9ab601ec ledctl-0.1.tgz
$ sha256sum ledctl-0.1-1.src.rpm
ee2d3db2c4e781a3423fc0e8e66206e837ee29e8a3484b234d5b51e51df9af53 ledctl-0.1-1.src.rpm
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.
GPLv2+
OK license is open source-compatible. License text included in package.
GPLv2+
OK latest version is being packaged.
OK BuildRequires are proper.
pod2man required for manpage generating, so BuildRequires: perl could be added for safety, however - perl is part of dependency tree of the basic buildtree so not required.
MAYBE compiler flags are appropriate.
make CFLAGS="$RPM_OPT_FLAGS --std=c99" - original Makefile uses hardcoded CFLAGS=-O0 -g -Wall -std=c99 - maybe you should remove this definition from Makefile if you want to use $RPM_OPT_FLAGS.
OK package builds in mock (Rawhide/i686).
OK debuginfo package looks complete.
BAD rpmlint is silent.
$ rpmlint ledctl.spec ledctl*.rpm
ledctl.spec: W: invalid-url Source0: whoknows.com/ledctl-0.1.tgz
ledctl-debuginfo.i686: W: no-url-tag
ledctl.i686: W: spelling-error %description -l en_US LEDs -> LED, LED's, LED s
ledctl.i686: W: no-url-tag
ledctl.src: W: spelling-error %description -l en_US LEDs -> LED, LED's, LED s
ledctl.src: W: no-url-tag
ledctl.src: W: invalid-url Source0: whoknows.com/ledctl-0.1.tgz
3 packages and 1 specfiles checked; 0 errors, 7 warnings.
Please, do not use whoknows.com in Source0 - it will produce errors in the mass checks. I would prefer comment in spec (explaining why there is no upstream URL and tarball download place)
OK final provides and requires look sane.
N/A %check is present and all tests pass.
N/A shared libraries are added to the regular linker search paths with
proper scriptlets
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
OK pkgconfig files in -devel
OK no libtool .la droppings.
OK not a GUI app.
Summary:
Change the Source0 - no whoknows.com please. Add explanation to the spec file. I can confirm there is no Intel Enclosure LED utilities product at http://downloadcenter.intel.com/default.aspx at the moment, however URL could at least point to Intel pages.
Consider this CFLAGS change.
Please resubmit the spec file and srpm and I'll check the "bad things" before I'll approve this package review.
new spec and packages uploaded. Fixed: MAYBE compiler flags are appropriate Change the Source0 - no whoknows.com please. URL could at least point to Intel pages rpmlint: ledctl.spec: W: invalid-url Source0: ledctl.tgz 1 packages and 1 specfiles checked; 0 errors, 1 warnings. Thanks, sha256sums of checked files: 08afcf3448cc9eb9b19a474367b81749731ca424aa28aa0178d1879c01eaec60 ledctl.spec 209a5430ffb166de65e9949f196e05be8da4937150997f1e509e27bd9ab601ec ledctl.tgz 99465542c9706fb4e2e6438e0ec40cb92e0a7666255048d22c7c76af01ca7c01 ledctl-0.1-1.src.rpm Looks sane for me now. APPROVED. New Package SCM Request ======================= Package Name: ledctl Short Description: Enclosure LED Utilities Owners: jmoskovc Branches: f14 el6 Git done (by process-git-requests). (In reply to comment #6) > Git done (by process-git-requests). Thank you! This package has been obsoleted by ledmon -> closing. |