Bug 528949

Summary: Review Request: drehatlas-widelands-fonts - Widelands font
Product: [Fedora] Fedora Reporter: Dave Ludlow <dave>
Component: Package ReviewAssignee: Dave Ludlow <dave>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, fonts-bugs, notting
Target Milestone: ---Flags: nicolas.mailhot: fedora-review+
kevin: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: 1.0.2.2-6.fc11 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2009-11-04 12:14:46 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 Dave Ludlow 2009-10-14 13:42:09 UTC
Spec URL: http://www.adsllc.com/tmp/oflb-Widelands-fonts.spec
SRPM URL: http://www.adsllc.com/tmp/oflb-Widelands-fonts-1.0.2.2-1.fc11.src.rpm
Description:
This font was created by Nasenbaer as a logo for the Widelands project, but
has since been enhanced.  The font is decorative and not really usable for
real texts, but it looks nice for logos, banners or similar stuff.  All normal
latin chars and arabic numbers are included and usable.

Comment 1 Dave Ludlow 2009-10-14 13:43:37 UTC
CC the fedora fonts bugs list

Comment 2 Dave Ludlow 2009-10-14 19:29:29 UTC
Koji scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1746547

$ rpmlint SRPMS/oflb-Widelands-fonts-1.0.2.2-1.fc11.src.rpm RPMS/noarch/oflb-Widelands-fonts-1.0.2.2-1.fc11.noarch.rpm 
2 packages and 0 specfiles checked; 0 errors, 0 warnings.

Comment 3 Nicolas Mailhot 2009-10-14 19:54:28 UTC
Please only use lowercase in the package name. Before F12 that's a packaging guidelines error, starting from F12 it'll also cause the build to fail  (since new code assumes packages respect naming guidelines)

http://fedoraproject.org/wiki/Fonts_packaging_policy#Naming

Comment 4 Nicolas Mailhot 2009-10-14 20:29:13 UTC
That being said, let's do a more thorough review

1. when creating a new file (such as the fontconfig rules one) it's better to use SourceX instead of patch, that produces a simple patch in the vcs people can look at or import. Using patch only adds lots of not-really useful +s before each line

2. likewise for simple operations such as re-wrapping text files it's better to script them instead of creating a trivial patch that will need to be rebased every few releases

I often use something like

for txt in *.txt; do
   fold -s $txt > $txt.new &&\
   sed -i 's/\r//' $txt.new &&\
   touch -r $txt $txt.new &&\
   mv $txt.new $txt
done

(you can remove the sed if you don't need to fix DOS EOLs, and add an iconv if necessary)

3. We sadly do not seem to have a standard script to invoque fontforge. So I can't say if your way is better or worse than others. It's probably a good idea to write a small makefile and get it included upstream so upstream at least will check it's happy with the way we generate the font. Someday we'll need to codify this so all our packages have the same bugs :)

4. you can put multiple files on a single %doc line

The rest seems fine (but I'll need a package buildable in rawhide to run our full battery of tests

Thank you for submitting this package, it's not perfect but it's also not too bad and you were awfully quick :)

Comment 5 Nicolas Mailhot 2009-10-14 20:42:27 UTC
And lastly (I'm not very efficient today, sorry) it's good style not to repeat the package name (or something similar) in the summary, since GUI and CLI tools already print the package name next to it. So you need to be a bit more creative

Comment 6 Dave Ludlow 2009-10-14 21:30:36 UTC
Renamed, patches replaced with SourceX's, Makefile used for conversion, %docs
combined.  Summary updated.  Waiting to hear back from upstream; will pass the
updates on when I do.

$ rpmlint oflb-widelands-fonts.spec oflb-widelands-fonts-1.0.2.2-2.fc11.src.rpm
oflb-widelands-fonts-1.0.2.2-2.fc11.noarch.rpm
2 packages and 1 specfiles checked; 0 errors, 0 warnings.

SPEC:
http://adsllc.fedorapeople.org/rpmbuild/SPECS/oflb-widelands-fonts.spec

Koji scratch build against f12:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1746690

Thanks for the review and helpful suggestions - I learned a few things.  Any
suggestions on getting a rawhide buildable package?

Comment 7 Dave Ludlow 2009-10-14 21:37:23 UTC
Fuedal->Feudal

SPEC replaced, Koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1746712

Comment 8 Nicolas Mailhot 2009-10-14 21:54:55 UTC
(In reply to comment #6)

> Thanks for the review and helpful suggestions - I learned a few things.  Any
> suggestions on getting a rawhide buildable package?  

lowercasing the package name should have been sufficient, I'll check it tomorrow (my time is up today)

Another point: oflb- in a font context means "a font downloaded from www.openfontlibrary.org" Since that does not seem the case for this package, you need to define another prefix

You have some documented examples here
http://fedoraproject.org/wiki/Fonts_packaging_policy#Naming

We prefix font package names by foundry to group packages with the same origin in package management tools, since an entity that produced good fonts will tend to publish other good fonts, and an entity that produced average fonts unfortunately rarely gets much better (or improves its old fonts at the same time). So this is useful to the user. Other SIGs maye have made similar choices (or not)

Comment 9 Nicolas Mailhot 2009-10-15 22:21:56 UTC
This one is good technically

I'd simplify the summary to
“A Latin typeface inspired by feudal calligraphy”

Also technically "glyph" is a better term than "char"

You didn't keep the touch after the fold, it's used to keep the original file timestamp (but that do not matter as much in recent rpm as it used to before, so this is just FYI)

But this is really nitpicking on my part. Ignore me if you don't want to bother

So the only remaining problem is the naming. Since the author publishes two other fonts, that may be packaged someday, finding a nice prefix to identify him is necessary (unfortunately renaming packages is one thing the Fedora infra is very bad at, so if you make a mistake here you'll be stuck with it quite a long time). pschwanemann- would be consistent with apanov- but a bit long. Feel free to use this or invent a better one (or ask upstream for a better handle)

Comment 10 Dave Ludlow 2009-10-16 13:52:09 UTC
Waiting on a response from upstream.  I would ideally like them to incorporate the font-building Makefile and either update the release on oflb or give their preferred foundry name.

Comment 11 Dave Ludlow 2009-10-16 16:59:23 UTC
Heard from upstream.  This attempt changes the name as suggested, adds the touch, and updates the font conversion to add .ttf output abilities in addition to .otf.  We don't actually use the .ttf stuff, but upstream may want it.

SPEC:
http://adsllc.fedorapeople.org/rpmbuild/SPECS/drehatlas-widelands-fonts.spec

Koji:
http://koji.fedoraproject.org/koji/taskinfo?taskID=1750409

This is a little premature - I'm waiting on a final confirmation from upstream before calling it official.

Comment 12 Nicolas Mailhot 2009-10-16 20:29:13 UTC
(In reply to comment #11)

> Koji:
> http://koji.fedoraproject.org/koji/taskinfo?taskID=1750409
> 
> This is a little premature - I'm waiting on a final confirmation from upstream
> before calling it official.

Well, this one is good

⨒⨒⨒ APPROVED ⨒⨒⨒

(Note that I'm not asking you to push it at once, just confirming this particular version is good enough for Fedora, and that you can push it or something similar when you feel ready. You can still wait for upstream confirmation or ask new questions here. But I don't think I should block you from now on)

When you feel ready, you can continue from
http://fedoraproject.org/wiki/Font_package_lifecycle#3.a

I hope the process was pleasant, and that it will inspire you to package a
other fonts for Fedora. Please do not hesitate to suggest improvements to our
organisation on the fonts mailing list.

Thank you for your contribution to our font package pool.

⇒ REASSIGNING

Comment 13 Dave Ludlow 2009-10-17 15:13:33 UTC
Upstream has confirmed - let's make it official.

New Package CVS Request
=======================
Package Name: drehatlas-widelands-fonts
Short Description: A Latin typeface inspired by feudal calligraphy
Owners: adsllc
Branches: F-10 F-11 F-12
InitialCC: fonts-sig

Comment 14 Nicolas Mailhot 2009-10-17 16:24:08 UTC
If upstream does not fix its “Decorative” buglet please do not change upstream's zip — write a fontforge script to patch the font, or look at
/usr/share/fontconfig/templates/remapping-font-template.*

Comment 15 Kevin Fenzi 2009-10-17 20:40:00 UTC
cvs done.

Comment 16 Fedora Update System 2009-10-18 02:46:06 UTC
drehatlas-widelands-fonts-1.0.2.2-6.fc11 has been submitted as an update for Fedora 11.
http://admin.fedoraproject.org/updates/drehatlas-widelands-fonts-1.0.2.2-6.fc11

Comment 17 Fedora Update System 2009-10-18 02:46:17 UTC
drehatlas-widelands-fonts-1.0.2.2-6.fc12 has been submitted as an update for Fedora 12.
http://admin.fedoraproject.org/updates/drehatlas-widelands-fonts-1.0.2.2-6.fc12

Comment 18 Fedora Update System 2009-10-18 02:46:26 UTC
drehatlas-widelands-fonts-1.0.2.2-6.fc10 has been submitted as an update for Fedora 10.
http://admin.fedoraproject.org/updates/drehatlas-widelands-fonts-1.0.2.2-6.fc10

Comment 19 Fedora Update System 2009-10-21 00:39:46 UTC
drehatlas-widelands-fonts-1.0.2.2-6.fc11 has been pushed to the Fedora 11 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update drehatlas-widelands-fonts'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F11/FEDORA-2009-10589

Comment 20 Fedora Update System 2009-10-21 00:39:58 UTC
drehatlas-widelands-fonts-1.0.2.2-6.fc10 has been pushed to the Fedora 10 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update drehatlas-widelands-fonts'.  You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2009-10593

Comment 21 Fedora Update System 2009-11-04 12:14:41 UTC
drehatlas-widelands-fonts-1.0.2.2-6.fc10 has been pushed to the Fedora 10 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 22 Fedora Update System 2009-11-04 12:14:53 UTC
drehatlas-widelands-fonts-1.0.2.2-6.fc11 has been pushed to the Fedora 11 stable repository.  If problems still persist, please make note of it in this bug report.