Bug 554530 - Review Request: cdf - A colorized df
Summary: Review Request: cdf - A colorized df
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Dominic Hopf
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-01-11 22:08 UTC by Damien Durand
Modified: 2010-05-28 17:26 UTC (History)
7 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-05-28 17:26:23 UTC
dmaphy: fedora-review+
dennis: fedora-cvs+


Attachments (Terms of Use)

Description Damien Durand 2010-01-11 22:08:32 UTC
Spec URL: http://splinux.fedorapeople.org/cdf/cdf.spec
SRPM URL: http://splinux.fedorapeople.org/cdf/cdf-0.2-1.fc12.src.rpm

Description: cdf means "colorized df". The main features of cdf are:

* customazable color schemes
* eye-friendly capacity bars
* most of such utils needs some 3rd party libraries, 
python interpreter and so on, while cdf written in pure C

Comment 1 Rich Mattes 2010-01-12 03:18:06 UTC
I'm not an approved packager yet, so I'll give you an informal review.

-rpmlint output is clean, and should be included in the review request:
$ rpmlint cdf.spec ../RPMS/i686/cdf-*
2 packages and 1 specfiles checked; 0 errors, 0 warnings.

- No koji builds?
- spec file looks ok
- Upstream source URL works
- Upstream archive md5sum matches md5sum of source in srpm
- Builds in mock (fedora-12-i386)
- License matches source files

Comments about the program itself:
1) Program installs and runs fine, but doesn't handle long names gracefully.  It pushes the columns out of line and line wraps, making the output inconsistent and not so eye-friendly.  Beyond the scope of packaging, but I thought I'd mention it.

2) It says the color scheme is customizable, but doesn't say how.  Maybe a manpage or readme could be included to explain it.

Comment 2 Damien Durand 2010-05-03 08:32:52 UTC
The configuration file is cdfrc.sample

Spec: http://splinux.fedorapeople.org/cdf/cdf.spec
SRPM: http://splinux.fedorapeople.org/cdf/cdf-0.2-2.fc13.src.rpm

Comment 3 Damien Durand 2010-05-14 13:18:25 UTC
Ping ? Sorry for the delay... :-)

Comment 4 Dominic Hopf 2010-05-14 14:02:23 UTC
I'll do the review next week. :)

Comment 5 Dominic Hopf 2010-05-22 13:45:04 UTC
$ rpmlint cdf.spec
0 packages and 1 specfiles checked; 0 errors, 0 warnings.


$ rpmlint /home/dmaphy/rpmbuild/SRPMS/cdf-0.2-2.fc12.src.rpm
cdf.src: W: spelling-error Summary(en_US) df -> sf, ff, dd
cdf.src: W: spelling-error %description -l en_US df -> sf, ff, dd
cdf.src: W: spelling-error %description -l en_US customazable -> customarily, customary, customization
cdf.src: W: spelling-error %description -l en_US utils -> utilizes, utilize, utility
1 packages and 0 specfiles checked; 0 errors, 4 warnings.

Translation suggestions:
========================
customazable -> customizable

* most of such utils needs some 3rd party libraries,
python interpreter and so on, while cdf written in pure C
->
* Most similar tools need 3rd party libraries, e.g. a python interpreter. cdf is
written in pure C.


$ rpmlint cdf-0.2-2.fc12.x86_64.rpm cdf-debuginfo-0.2-2.fc12.x86_64.rpm
cdf.x86_64: W: spelling-error Summary(en_US) df -> sf, ff, dd
cdf.x86_64: W: spelling-error %description -l en_US df -> sf, ff, dd
cdf.x86_64: W: spelling-error %description -l en_US customizable -> customization, customize, customarily
cdf.x86_64: W: spelling-error %description -l en_US utils -> utilizes, utilize, utility
cdf.x86_64: W: no-manual-page-for-binary cdf
2 packages and 0 specfiles checked; 0 errors, 5 warnings.

For the wording, see the suggestions above. For the manpage, please contact
upstream.


Package Review
==============

Key:
 - = N/A
 x = Check
 ! = Problem
 ? = Not evaluated

=== REQUIRED ITEMS ===
 [x] Package is named according to the Package Naming Guidelines
 [x] Specfile name matches %{name}.spec
 [x] Package seems to meet Packaging Guidelines
 [x] Package successfully compiles and builds into binary RPMs on at least one
     supported architecture.
     Tested on: Fedora 12/x86_64
 [x] Rpmlint output:
     source RPM: see above
     binary RPM: see above
 [x] Package is not relocatable.
 [x] License in specfile matches actual License and meets Licensing Guidelines
     License: GPLv2+
 [x] License file is included in %doc.
 [x] Specfile is legible and written in AE
     There are some words which get claimed by rpmlint and a spellchecker, but
     a dictionary actually told me they are okay.

 [x] Sourcefile in the Package is the same as provided in the mentioned Source
     SHA1SUM of Source: 5f5d0c1f1003d9ad3c3cbbda1d8159e9fe10768a
 [x] Package compiles successfully
 [-] All build dependencies are listed in BuildRequires
 [-] Specfile handles locales properly
 [-] ldconfig called in %post and %postun if required
 [x] Package owns directorys it creates
 [-] Package requires other packages for directories it uses.
 [x] Package does not list a file more than once in the %files listing
 [x] %files section includes %defattr and permissions are set properly
 [x] %clean section is there and contains rm -rf $RPM_BUILD_ROOT
 [x] Macros are consistently used
 [x] Package contains code, or permissable content.
 [-] Large documentation files are in a -doc subpackage
 [x] Program runs properly without files listed in %doc
 [-] Header files are in a -devel package
 [-] Static libraries are in a -static package
 [-] Package requires pkgconfig if .pc files are present
 [-] .so-files are put into a -devel subpackage
 [-] Subpackages include fully versioned dependency for the base package
 [-] Any libtool archives (*.la) are removed
 [-] contains desktop file (%{name}.desktop) if it is a GUI application
 [x] Package does not own files or directories owned by other packages.
 [x] $RPM_BUILD_ROOT is removed at beginning of %install
 [-] Filenames are encoded in UTF-8

=== SUGGESTED ITEMS ===
 [x] Package contains latest upstream version
 [x] Package does not include license text files separate from upstream.
 [-] non-English translations for description and summary
 [x] Package builds in mock
     Tested on: F12/x86_64
 [x] Package should compile and build into binary RPMs on all supported architectures.
     tested build with koji
 [x] Program runs
 [-] Scriptlets must be sane, if used.
 [-] pkgconfig (*.pc) files are placed in a -devel package
 [-] require package providing a file instead of the file itself
     no files outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin are required

=== Issues to be point out ===
The make command lacks the %{optflags} macro. I guess this is intended, since
the package does not build with the optflags. You should add at least a comment,
why you didn't use the optflags macro.

You can use the %{name} macro in line 41.

I can fully reproduce the two issues Rich mentioned. They shall not block this
review, but it would be really nice if you would report them to upstream if
you didn't yet.

The two issues I mentioned don't block the review neither. Anything else looks
great, your package is APPROVED.

Comment 6 Damien Durand 2010-05-23 16:36:12 UTC
New Package CVS Request
=======================
Package Name: cdf
Short Description: cdf - A colorized df
Owners: splinux
Branches: F-11 F-12 F-13

Comment 7 Dennis Gilmore 2010-05-25 20:57:04 UTC
CVS Done Except for F-11 which does not accept new branches


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