Bug 202356 - Review Request: terminus-font - Clean fixed width font
Review Request: terminus-font - Clean fixed width font
Status: CLOSED CURRENTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Kevin Fenzi
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT 486248
  Show dependency treegraph
 
Reported: 2006-08-13 12:12 EDT by Hans Ulrich Niedermann
Modified: 2009-02-18 21:07 EST (History)
1 user (show)

See Also:
Fixed In Version: 4.20-5.fc5
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-05 06:24:05 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Hans Ulrich Niedermann 2006-08-13 12:12:07 EDT
Spec URL: http://n-dimensional.de/software/terminus-font/terminus-font.spec
SRPM URL: http://n-dimensional.de/software/terminus-font/terminus-font-4.20-3.src.rpm
Description: Clean fixed width font

The Terminus Font is designed for long (8 and more hours per day) work
with computers.

Version 4.16 contains 690 characters, covering code pages
ISO8859-1/2/5/9/13/15/16, IBM-437/852/855/866, KOI8-R/U/E/F,
Windows-1250/1251/1252/1254/1257, Paratype-PT154/PT254, Bulgarian-MIK,
Macintosh-Ukrainian, Esperanto and many others (a total of about 110
language sets). Also included are the IBM VGA, vt100 and xterm
pseudographic characters.

The sizes present are 6x12, 8x14, 8x16, 10x20, 12x24, 14x28 and
16x32. The styles are normal and bold (except for 6x12), plus
EGA/VGA-bold for 8x14 and 8x16.

The font is available for the Linux console and for X11.

This is my first package for Fedora, so I'm going to need a sponsor.
Comment 1 Hans Ulrich Niedermann 2006-08-13 12:27:21 EDT
A few notes on the package:

 * rpmlint reports no problems.
 * The package name is derived from the upstream tarball name "terminus-font". 
However, there is no binary package "terminus-font", only the two subpackages
"terminus-font-console" and "terminus-font-x11".
 * The spec file hopefully meets the Packaging Guidelines. There are no specific
Packaging Guidelines for fonts, therefore a little guesswork and copying from
other packages had to happen.
 * The font is licensed under the GPL.
 * It builds with mock for FC5 and FC6.
Comment 2 Kevin Fenzi 2006-08-27 12:43:57 EDT
Hey Hans. I will take a crack at reviewing this package and possibly sponsoring 
you. 

I should get a full review out on it hopefully later today. 

In the mean time, you may want to take a look at: 
http://fedoraproject.org/wiki/Extras/HowToGetSponsored
and perhaps add some comments to other pending reviews. 
Comment 3 Kevin Fenzi 2006-08-27 14:37:50 EDT
OK - Package name
OK - Spec file matches base package name.
OK - Meets Packaging Guidelines.
OK - License (GPL)
OK - License field in spec matches
See below - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
fe9d8e25b9537f6b3154d07d3da50375  terminus-font-4.20.tar.gz
fe9d8e25b9537f6b3154d07d3da50375  terminus-font-4.20.tar.gz.1
OK - Package compiles and builds on at least one arch.
n/a - Package needs ExcludeArch
OK - BuildRequires correct
n/a - Spec handles locales/find_lang
n/a - Spec has needed ldconfig in post and postun
n/a - Package is relocatable and has a reason to be.
OK - Package owns all the directories it creates.
OK - Package has no duplicate files in %files.
OK - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Spec has consistant macro usage.
OK - Package is code or permissible content.
n/a - -doc subpackage needed/used.
OK - Packages %doc files don't affect runtime. 
n/a - Headers/static libs in -devel subpackage.
n/a - .pc files in -devel subpackage.
n/a - .so files in -devel subpackage.
n/a - -devel package Requires: %{name} = %{version}-%{release}
n/a - .la files are removed.
n/a - Package is a GUI app and has a .desktop file
OK - Package doesn't own any directories other packages own.
See below - No rpmlint output.

SHOULD Items:

See below - Should include License or ask upstream to include it.
OK - Should build in mock.
See below - Should have sane scriptlets.

Issues:

1. Should ping upstream to include a copy of the GPL COPYING file.

2. The Group doesn't seem right here. Other font packages use:
User Interface/X
(Granted that rpm groups aren't very usefull, but we should try and
be consistant at least).

3. perl and gawk are in the default BuildRequires, no need to list them.

4. rpmlint says:

W: terminus-font-x11 dangerous-command-in-%preun rm

Why remove those files?
Comment 4 Mamoru TASAKA 2006-08-27 20:02:11 EDT
(In reply to comment #3)

> W: terminus-font-x11 dangerous-command-in-%preun rm
> 
> Why remove those files?

Hello, Hans:
You have to take a "%ghost file" method.

i.e.
In install stage: 
  touch $RPM_BUILD_ROOT%{local_x11_font_dir}/fonts.dir etc
In file entry:    
 %ghost %verify(not md5 size mtime) %{local_x11_font_dir}/fonts.dir etc
... and get rid of rm command from %preun entry (I think that the
    whole %preun stage is unnecessary).
Comment 5 Hans Ulrich Niedermann 2006-08-31 12:42:12 EDT
Adressing the issues raised in comment #3:

1. Will do so.

2. As per your wish, I have changed the groups for both the source package and
the -console subpackage (which is completely unrelated to X11) from
"Application/Text" to "User Interface/X". The -x11 subpackage has always been
"User Interface/X", so that has not changed.

3. Removed perl and awk dependencies.

4. mkfontdir (called by %post) creates these files. As they are in a directory
owned by this package, I want "rpm -e" to completely remove the directory
including any content the package may have placed there. However, comment #4
seems to suggest the proper way to do that, which I did not know about probably
due to the lack of documentation.

WRT comment #4:

Thanks a lot, this looks to be exactly what needs to be done here.

I have just uploaded 4.20-5 with these changes incorporated.
Comment 6 Kevin Fenzi 2006-08-31 22:17:26 EDT
2. Ah, I missed that that was the console fonts package in that group. 
Looking around I can't seem to find any other packages that provide console 
fonts, so it's hard to say what group it should be in. :(
I guess your "Application/Text" might be better than "User Interface/X" 
if you want to change it back. 

All the rest look fixed up...

This package looks like it's good to be APPROVED. 

I would be happy to sponsor you... 
You should be able to continue the process from the 
"Get a Fedora Account" step on: 
http://www.fedoraproject.org/wiki/Extras/Contributors

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