Red Hat Bugzilla – Bug 528949
Review Request: drehatlas-widelands-fonts - Widelands font
Last modified: 2009-11-04 07:14:59 EST
Spec URL: http://www.adsllc.com/tmp/oflb-Widelands-fonts.spec
SRPM URL: http://www.adsllc.com/tmp/oflb-Widelands-fonts-126.96.36.199-1.fc11.src.rpm
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.
CC the fedora fonts bugs list
Koji scratch build:
$ rpmlint SRPMS/oflb-Widelands-fonts-188.8.131.52-1.fc11.src.rpm RPMS/noarch/oflb-Widelands-fonts-184.108.40.206-1.fc11.noarch.rpm
2 packages and 0 specfiles checked; 0 errors, 0 warnings.
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)
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
(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 :)
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
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-220.127.116.11-2.fc11.src.rpm
2 packages and 1 specfiles checked; 0 errors, 0 warnings.
Koji scratch build against f12:
Thanks for the review and helpful suggestions - I learned a few things. Any
suggestions on getting a rawhide buildable package?
SPEC replaced, Koji:
(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
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)
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)
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.
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.
This is a little premature - I'm waiting on a final confirmation from upstream before calling it official.
(In reply to comment #11)
> 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
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.
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
Branches: F-10 F-11 F-12
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
drehatlas-widelands-fonts-18.104.22.168-6.fc11 has been submitted as an update for Fedora 11.
drehatlas-widelands-fonts-22.214.171.124-6.fc12 has been submitted as an update for Fedora 12.
drehatlas-widelands-fonts-126.96.36.199-6.fc10 has been submitted as an update for Fedora 10.
drehatlas-widelands-fonts-188.8.131.52-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
drehatlas-widelands-fonts-184.108.40.206-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
drehatlas-widelands-fonts-220.127.116.11-6.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.
drehatlas-widelands-fonts-18.104.22.168-6.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report.