Bug 682353

Summary: Review Request: wmtop - Mini graphical version of the CPU monitoring utility top
Product: [Fedora] Fedora Reporter: Mario Blättermann <mario.blaettermann>
Component: Package ReviewAssignee: Martin Gieseking <martin.gieseking>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, martin.gieseking, notting, raghusiddarth
Target Milestone: ---Flags: martin.gieseking: fedora-review+
j: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: wmtop-0.84-3.fc14 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-03-27 03:09:22 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 Mario Blättermann 2011-03-04 21:59:15 UTC
Spec URL: http://dl.dropbox.com/u/19373040/Fedora/SPECS/wmtop.spec
SRPM URL: http://dl.dropbox.com/u/19373040/Fedora/wmtop-0.84-1.fc14.src.rpm
Description: Dockapp with a mini graphical version of the cpu monitoring utility top. Wmtop maintains a view of the 3 top CPU (or memory) consuming processes displaying the amount of CPU used as a horizontal bar.

Comment 1 Martin Gieseking 2011-03-05 08:46:48 UTC
Some initial comments:

- The proper project site of wmtop seems to be http://wmtop.sourceforge.net. 
  You should adapt URL and Source0 accordingly.

- Replace CFLAGS with OPTS in the make statement. Otherwise, the %{optflags} 
  don't get applied. 

- Add -p to "install" in order to preserve the timestamps, especially that of 
  the manpage. 

- Replace %{name}.1.gz with %{name}.1* since you should not rely on a specific
  compression format (as it may change).

Comment 2 Mario Blättermann 2011-03-05 10:38:48 UTC
Spec URL: http://dl.dropbox.com/u/19373040/Fedora/SPECS/wmtop.spec
SRPM URL: http://dl.dropbox.com/u/19373040/Fedora/wmtop-0.84-2.fc14.src.rpm

(In reply to comment #1)
> Some initial comments:
> 
> - The proper project site of wmtop seems to be http://wmtop.sourceforge.net. 
>   You should adapt URL and Source0 accordingly.
The project page is still present, but the download location is down. It's a common problem with a lot of older dockapps. That's why it is a good choice to link to the pool at dockapps.org here
> 
> - Replace CFLAGS with OPTS in the make statement. Otherwise, the %{optflags} 
>   don't get applied.
Done.
> 
> - Add -p to "install" in order to preserve the timestamps, especially that of 
>   the manpage. 
>
Done.
> - Replace %{name}.1.gz with %{name}.1* since you should not rely on a specific
>   compression format (as it may change).
Done.

Comment 3 Martin Gieseking 2011-03-05 10:50:56 UTC
(In reply to comment #2)
> The project page is still present, but the download location is down.

Hm, the corresponding file section at SourceForge is still present. It's just the links that are outdated. You can download the tarball via the usual SF URL:
http://downloads.sourceforge.net/wmtop/wmtop-0.84.tar.bz2

Comment 4 Mario Blättermann 2011-03-05 11:01:53 UTC
Spec URL: http://dl.dropbox.com/u/19373040/Fedora/SPECS/wmtop.spec
SRPM URL: http://dl.dropbox.com/u/19373040/Fedora/wmtop-0.84-3.fc14.src.rpm

(In reply to comment #3)
> Hm, the corresponding file section at SourceForge is still present. It's just
> the links that are outdated. You can download the tarball via the usual SF URL:
> http://downloads.sourceforge.net/wmtop/wmtop-0.84.tar.bz2

Changed.

Comment 5 Mario Blättermann 2011-03-05 11:15:11 UTC
FYI, the Koji scratch build is here:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2886866

Comment 6 Raghu Udiyar 2011-03-07 23:18:12 UTC
Hi,

This is an informal review per the package guidelines given here: http://fedoraproject.org/wiki/Packaging:Guidelines


MUST ITEMS

[-] rpmlint output

* Just some spelling errors (rpmlint -i will give more details) :

$ rpmlint wmtop.spec ../RPMS/x86_64/wmtop-* ../SRPMS/wmtop-0.84-3.fc14.src.rpm 
wmtop.x86_64: W: spelling-error Summary(de) top -> Top, top-, -top
wmtop.x86_64: W: spelling-error %description -l en_US Dockapp -> Dock app, Dock-app, Dockage
wmtop.x86_64: W: spelling-error %description -l en_US cpu -> CPU, cup, cp
wmtop.x86_64: W: spelling-error %description -l de Dockapp -> Dock, Andocke, Documenta
wmtop.x86_64: W: spelling-error %description -l de top -> Top, top-, -top
wmtop.src: W: spelling-error Summary(de) top -> Top, top-, -top
wmtop.src: W: spelling-error %description -l en_US Dockapp -> Dock app, Dock-app, Dockage
wmtop.src: W: spelling-error %description -l en_US cpu -> CPU, cup, cp
wmtop.src: W: spelling-error %description -l de Dockapp -> Dock, Andocke, Documenta
wmtop.src: W: spelling-error %description -l de top -> Top, top-, -top
3 packages and 1 specfiles checked; 0 errors, 10 warnings.


[x] The package is named according to the Package Naming Guidelines
[x] The spec file name matches the base package %{name}, in the format
%{name}.spec
[x] The package meets the Packaging Guidelines.
[x] The package is licensed with a Fedora approved license and meet the
Licensing Guidelines . (GPLv2+)
[x] The License field in the package spec file matches the actual license.
[x] The source package includes the text of the license(s) in its own file,
then that file, containing the text of the license(s) for the package is
included in %doc.
[x] The spec file is written in American English.
[x] The spec file for the package is legible.
[x] The sources used to build the package matches the upstream source, as
provided in the spec URL. (md5: 2bab22c5bc3a5b887e7c03d6dbfe59d7 )
[x] The package successfully compiles and builds into binary rpms on two
primary architectures (http://koji.fedoraproject.org/koji/taskinfo?taskID=2886866)
[x] All build dependencies are listed in BuildRequires
[x] No locale files.
[x] Packages does NOT bundle copies of system libraries.
[x] Package is not designed to be relocatable.
[x] A package  owns all directories that it creates.
[x] No  file  listed more than once in the spec file's %files listings. 
[x] Permissions on files are set properly. Executables are set with executable
permissions. Every %files section includes a %defattr(...) line.
[x] Each package has consistently use macros.
[x] The package contains codes, or permissable content.
[x] There is no large documentation files
[x] The program runs properly even if %doc it is not present
[x] Packages does not own files or directories already owned by other packages.
[x] All filenames in rpm packages are valid UTF-8.


SHOULD ITEMS

[x] Source package includes license text(s) as a separate file from upstream.
[X] The description and summary sections in the package spec file does not 
contain translations for supported Non-English languages, if available. (German traslation provided)
[x] Tested the package was built fine in mock.
[?] The package compiles and build into binary rpms on all supported
architectures. (Tested on x86_64 and i386 successfully)
[X] A package does not segfault. (Did not segfault for me)
[x] No scriptlet used
[x] No subpackage used
[x] No pkgconfig(.pc) used
[x] No file dependencies
[x] Manpage included

Comment 7 Mario Blättermann 2011-03-11 23:14:20 UTC
Thanks for your review. If there are no other issues, would you assign this bug to you please and approve the package? Many thanks.

Comment 8 Martin Gieseking 2011-03-12 06:26:37 UTC
Since Raghu did an informal review, he's probably not sponsored yet and can't approve the package (requires a formal review). If nobody else volunteers, I'll take this ticket in the next few days.

Comment 9 Martin Gieseking 2011-03-20 19:54:49 UTC
The package looks good. I'd just replace "cpu" with "CPU" as suggested by the spell checker. That's not a blocker though.


$ rpmlint /var/lib/mock/fedora-14-x86_64/result/*.rpm
wmtop.src: W: spelling-error Summary(de) top -> Top, top-, -top
wmtop.src: W: spelling-error %description -l en_US Dockapp -> Dock app, Dock-app, Dockage
wmtop.src: W: spelling-error %description -l en_US cpu -> CPU, cup, cp
wmtop.src: W: spelling-error %description -l de Dockapp -> Dock, Andocke, Documenta
wmtop.src: W: spelling-error %description -l de top -> Top, top-, -top
wmtop.x86_64: W: spelling-error Summary(de) top -> Top, top-, -top
wmtop.x86_64: W: spelling-error %description -l en_US Dockapp -> Dock app, Dock-app, Dockage
wmtop.x86_64: W: spelling-error %description -l en_US cpu -> CPU, cup, cp
wmtop.x86_64: W: spelling-error %description -l de Dockapp -> Dock, Andocke, Documenta
wmtop.x86_64: W: spelling-error %description -l de top -> Top, top-, -top
3 packages and 0 specfiles checked; 0 errors, 10 warnings.


---------------------------------
key:

[+] OK
[.] OK, not applicable
[X] needs work
---------------------------------

[+] MUST: The package must be named according to the Package Naming Guidelines.
[+] MUST: The spec file name must match the base package %{name}.
[+] MUST: The package must meet the Packaging Guidelines.
[+] MUST: The package must be licensed with a Fedora approved license.
    - GPLv2+ according to source file headers

[+] MUST: The License field in the package spec file must match the actual license.
[+] MUST: The file containing the text of the license(s) for the package must be included in %doc.
[+] MUST: The spec file must be written in American English.
[+] MUST: The spec file for the package MUST be legible.
[+] MUST: The sources used to build the package must match the upstream source.
    $ md5sum wmtop-0.84.tar.bz2*
    2bab22c5bc3a5b887e7c03d6dbfe59d7  wmtop-0.84.tar.bz2
    2bab22c5bc3a5b887e7c03d6dbfe59d7  wmtop-0.84.tar.bz2.1

[+] MUST: The package MUST successfully compile and build into binary rpms on at least one primary architecture.
[.] MUST: If the package does not successfully compile, build or work on an architecture, ...
[+] MUST: All build dependencies must be listed in BuildRequires.
[+] MUST: When compiling C, C++, or Fortran files, %{optflags} must be applied.
[.] MUST: The spec file MUST handle locales properly.
[.] MUST: Packages storing shared library files (not just symlinks) must call ldconfig in %post and %postun.
[+] MUST: Packages must NOT bundle copies of system libraries.
[.] MUST: If the package is designed to be relocatable, ...
[+] MUST: A package must own all directories that it creates. 
[+] MUST: A Fedora package must not list a file more than once in %files.
[+] MUST: Permissions on files must be set properly.
[+] MUST: Each package must consistently use macros.
[+] MUST: The package must contain code, or permissable content.
[.] MUST: Large documentation files must go in a -doc subpackage.
[+] MUST: Files in %doc must not affect the runtime of the application.
[.] MUST: Header files must be in a -devel package.
[.] MUST: Static libraries must be in a -static package.
[.] MUST: If a package contains library files with a suffix (e.g. libfoo.so.1.1), ...
[.] MUST: devel packages must require the base package using a fully versioned dependency.
[.] MUST: Packages must NOT contain any .la libtool archives.
[.] MUST: Packages containing GUI applications must include a %{name}.desktop file
[+] MUST: Packages must not own files or directories already owned by other packages.
[+] MUST: All filenames in rpm packages must be valid UTF-8.

[.] SHOULD: If the source package does not include license text(s) as a separate file from upstream,...
[+] SHOULD: Timestamps of files should be preserved.
[+] SHOULD: The reviewer should test that the package builds in mock.
[+] SHOULD: The package should compile and build into binary rpms on all supported architectures.
[+] SHOULD: The reviewer should test that the package functions as described.
[.] SHOULD: If scriptlets are used, those scriptlets must be sane.
[.] SHOULD: Usually, subpackages other than devel should require the base package using a fully versioned dependency.
[.] SHOULD: pkgconfig(.pc) files should be placed in a -devel pkg.
[.] SHOULD: If the package has file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin consider requiring the package which provides the file instead of the file itself.
[+] SHOULD: Your package should contain man pages for binaries/scripts.

----------------
Package APPROVED
----------------

Comment 10 Mario Blättermann 2011-03-21 18:55:20 UTC
(In reply to comment #9)
> The package looks good. I'd just replace "cpu" with "CPU" as suggested by the
> spell checker. That's not a blocker though.
> 
OK, I will change it before I feed Koji with the Git content. Many thanks for your review.

Comment 11 Mario Blättermann 2011-03-21 18:59:42 UTC
New Package SCM Request
=======================
Package Name: wmtop
Short Description: Mini graphical version of the CPU monitoring utility top
Owners: mariobl
Branches: f14 f15
InitialCC: mariobl

Comment 12 Jason Tibbitts 2011-03-21 21:54:48 UTC
Git done (by process-git-requests).

Comment 13 Fedora Update System 2011-03-22 20:42:09 UTC
wmtop-0.84-3.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/wmtop-0.84-3.fc15

Comment 14 Fedora Update System 2011-03-22 20:42:18 UTC
wmtop-0.84-3.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/wmtop-0.84-3.fc14

Comment 15 Fedora Update System 2011-03-23 04:44:54 UTC
wmtop-0.84-3.fc15 has been pushed to the Fedora 15 testing repository.

Comment 16 Fedora Update System 2011-03-27 03:09:17 UTC
wmtop-0.84-3.fc15 has been pushed to the Fedora 15 stable repository.

Comment 17 Fedora Update System 2011-04-02 22:50:54 UTC
wmtop-0.84-3.fc14 has been pushed to the Fedora 14 stable repository.