Please do not reply directly to this email. All additional comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=461139
Nicolas Mailhot nicolas.mailhot@laposte.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |besfahbo@redhat.com AssignedTo|nobody@fedoraproject.org |nicolas.mailhot@laposte.net Flag| |fedora-review?, | |needinfo?(440volt.tux@gmail | |.com)
--- Comment #23 from Nicolas Mailhot nicolas.mailhot@laposte.net 2008-10-26 08:26:27 EDT --- I wanted to wait for a submission with a fixed name, but since I'm doing a font review run today anyway, here is a full review.
(on http://subhodip.fedorapeople.org/Thabit-fonts-0.02-3.fc9.src.rpm)
1. Please rename package to arabeyes-thabit-fonts
2. You have a typo in Summary
3. Is upstream's URL http://www.arabeyes.org/ or http://www.arabeyes.org/project.php?proj=Khotot (as referenced in the README) ?
4. Is the description still ok for a Thabit-only package?
5. It's harmless, but a Source2 without Source1 is strange, especially on a new package
6. Please use normal %setup macro in your package. That will simplify the spec a lot (your complex manual unpacking was only necessary because you initially tried to stuff two fonts in a single package)
7. Since upstream uses fontforge for building, please ask upstream to publish sfd sources and build fedora fonts from those sources (we have many packages that do so, for example DejaVu, Liberation, Inconsolata, etc)
8. Some people want all Fedora-added source files in a package to be prefixed with the package name. You don't follow this convention for your fontconfig file. Please use %{name}-fontconfig.conf as suggested by http://fedoraproject.org/wiki/Packaging/FontsSpecTemplate
9. I don't know if 65 is the right fontconfig priority for this font. You need to discuss it with Behdad (preferably CC-ing the fedora fonts list)
10. Please only declare thabit-related fontconfig rules in the fontconfig file shipped with the thabit package
11. You probably want a "Generic name" rule in addition to the "Registering a font in default families" rule in your fontconfig file → http://fedoraproject.org/wiki/Fontconfig_packaging_tips#Generic_names This rule is used by fontconfig to complete your font with glyphs from other fonts when it encounters a codepoint your font is missing → http://fedoraproject.org/wiki/Fontconfig_packaging_tips#Registering_a_font_i... This rule is used by fontconfig to identify what fonts to use when an application requests a "cursive" font.
12. Please reformat your xml files with xmllint --format before submission so they are nicely indented with the same rules as other font packages
13. When you've written fontconfig rules you're happy with it's always a good idea to send them upstream to be included in the font next releases (in the versionned source archive you're supposed to request)
14. You can drop the -f flag to fc-cache for releases ≥ Fedora 9
15. OFL.txt mentions an IBM copyright. Please ask upstream to add a Fontlog.txt to their source release archive (as recommended by SIL for OFL fonts), that clearly identifies what other fonts were used to create this font, so Fedora Legal can check there is no problem
16. Please make sure all the steps up to 2.a (included) have been followed in https://fedoraproject.org/wiki/Font_package_lifecycle In particular we need a wiki page that describes the font to be added to the wiki (for release notes)
Well that's a lot of stuff to fix and it's a pity the OLPC folks didn't do a full review before accepting this font package. I'm putting a "NEEDINFO reporter" on this bug and the Mothanna one (since it needs more or less the same fixes). Please remove this flag once you've fixed the previous points.
Please don't hesitate to ask questions on the fedora fonts list if there are elements you need help with.
fonts-bugs@lists.fedoraproject.org
-
Red Hat Bugzilla