Bug 700862 - Review Request: wmcalc - Calculator in a WindowMaker dockapp
Summary: Review Request: wmcalc - Calculator in a WindowMaker dockapp
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Gwyn Ciesla
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-04-29 16:02 UTC by Mario Blättermann
Modified: 2011-08-02 02:04 UTC (History)
4 users (show)

Fixed In Version: wmcalc-0.3-4.fc15
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-08-02 02:04:49 UTC
Type: ---
Embargoed:
gwync: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Mario Blättermann 2011-04-29 16:02:02 UTC
Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/wmcalc.spec
SRPM URL: http://mariobl.fedorapeople.org/Review/SRPMS/wmcalc-0.3-1.fc14.src.rpm

Description:
Wmcalc is a 64x64 pixel application that performs all the functions (and
eventually more) of a simple four function calculator. It includes a 10 digit
alpha-numeric display, and twenty buttons for user input. Clicking on the
display will clear the calculator.
It is specifically designed to be docked in Windowmaker, or 'Swallowed'
by wharf in Afterstep. Of course, it should work in just about any
window manager.

Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3035955

Well, it builds, but the debug package is still useless (debuginfo-without-sources). I've applied the variable CFLAGS="$RPM_OPT_FLAGS" to the "make" call, but without success.

Comment 1 Martin Gieseking 2011-04-29 17:22:14 UTC
(In reply to comment #0)
> Well, it builds, but the debug package is still useless
> (debuginfo-without-sources). I've applied the variable CFLAGS="$RPM_OPT_FLAGS"
> to the "make" call, but without success.

As you can see in the Makefile, CFLAGS is not used but FLAGS is. So just replace the variable and the %optflags should be applied.

If you want to add the configuration file with %doc, remove the leading dot as there shouldn't be any hidden doc files.

BTW, there is a memory issue in the application (array index out of bounds) that might lead to a segfault on some systems. The number 11 should be replaced with DISPSIZE in lines 117 and 118 of file wmcalc.c. If the upstream project is dead, you might want to add a corresponding patch yourself.

Comment 2 Mario Blättermann 2011-04-29 19:50:51 UTC
Thanks for your hints.

Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/wmcalc.spec
SRPM URL:
http://mariobl.fedorapeople.org/Review/SRPMS/wmcalc-0.3-2.fc14.src.rpm

In case of the hidden file, I've renamed it to "wmcalc-config" to make somewhat clearer what it is for.

Comment 3 Mario Blättermann 2011-04-29 19:54:53 UTC
Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=3036469

Comment 4 Gwyn Ciesla 2011-07-18 13:17:59 UTC
Good:

- rpmlint checks return:

wmcalc.x86_64: W: spelling-error Summary(en_US) dockapp -> dock app, dock-app, paddock
The value of this tag appears to be misspelled. Please double-check.

Ignore.

wmcalc.x86_64: E: incorrect-fsf-address /usr/share/doc/wmcalc-0.3/COPYING
The Free Software Foundation address in this file seems to be outdated or
misspelled.  Ask upstream to update the address, or if this is a license file,
possibly the entire file with a new copy available from the FSF.

Fix, probably just need a new copy of the file.

wmcalc.x86_64: W: no-manual-page-for-binary wmcalc
Each executable in standard binary directories should have a man page.

Include if available.

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

- package meets naming guidelines
- package meets packaging guidelines
- license ( GPLv2+ ) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86_64)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file 


Fix:  Change license tag to GPLv2+

Question:  Does this or does this not need a .desktop file?

Mock build in progress to double-check BRs.

Comment 5 Gwyn Ciesla 2011-07-18 13:32:29 UTC
Mock build was good, so it's just the license tag and .desktop question.

Comment 6 Mario Blättermann 2011-07-18 18:12:48 UTC
(In reply to comment #4)

> wmcalc.x86_64: E: incorrect-fsf-address /usr/share/doc/wmcalc-0.3/COPYING
> The Free Software Foundation address in this file seems to be outdated or
> misspelled.  Ask upstream to update the address, or if this is a license file,
> possibly the entire file with a new copy available from the FSF.
> 
> Fix, probably just need a new copy of the file.
>
Fixed,I've taken a new license file from the FSF website.

> wmcalc.x86_64: W: no-manual-page-for-binary wmcalc
> Each executable in standard binary directories should have a man page.
> 
> Include if available.
> 
I found a manpage in an Ubuntu package, which has been written originally for Debian: http://manpages.ubuntu.com/manpages/gutsy/man1/wmcalc.1.html
 
[...]
> 
> Fix:  Change license tag to GPLv2+
> 
Fixed.

> Question:  Does this or does this not need a .desktop file?
> 
We don't need a *.desktop file for WindowMaker dockapps. See https://bugzilla.redhat.com/show_bug.cgi?id=701079#c5.

New files:
Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/wmcalc.spec
SRPM URL:
http://mariobl.fedorapeople.org/Review/SRPMS/wmcalc-0.3-3.fc15.src.rpm

Thanks for your time you spent on this package.

Comment 7 Gwyn Ciesla 2011-07-18 18:16:43 UTC
Excellent, one problem, but I trust you'll fix it before importing:

wmcalc.x86_64: W: spurious-executable-perm /usr/share/man/man1/wmcalc.1.gz
The file is installed with executable permissions, but was identified as one
that probably should not be executable.  Verify if the executable bits are
desired, and remove if not.

Given that, APPROVED.

Comment 8 Gwyn Ciesla 2011-07-18 18:17:49 UTC
Oh, and one more thing, you might want to rename Source2 to wmcalc-gpl-2.0.txt, to avoid SRPM clobbering.

Comment 9 Mario Blättermann 2011-07-22 13:05:09 UTC
Many thanks for your review! I've changed the files once again to reflect your review:

Spec URL: http://mariobl.fedorapeople.org/Review/SPECS/wmcalc.spec
SRPM URL:
http://mariobl.fedorapeople.org/Review/SRPMS/wmcalc-0.3-4.fc15.src.rpm

Comment 10 Mario Blättermann 2011-07-22 13:07:26 UTC
New Package SCM Request
=======================
Package Name: wmcalc
Short Description: Calculator in a WindowMaker dockapp
Owners: mariobl
Branches: f15

Comment 11 Gwyn Ciesla 2011-07-22 13:09:21 UTC
Git done (by process-git-requests).

Comment 12 Fedora Update System 2011-07-22 20:45:42 UTC
wmcalc-0.3-4.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/wmcalc-0.3-4.fc15

Comment 13 Fedora Update System 2011-07-23 01:55:24 UTC
wmcalc-0.3-4.fc15 has been pushed to the Fedora 15 testing repository.

Comment 14 Fedora Update System 2011-08-02 02:04:44 UTC
wmcalc-0.3-4.fc15 has been pushed to the Fedora 15 stable repository.


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