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=479596 Nicolas Mailhot <nicolas.mailhot@xxxxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- AssignedTo|nicolas.mailhot@xxxxxxxxxxx |oget.fedora@xxxxxxxxx Flag|fedora-review? |fedora-review+ --- Comment #4 from Nicolas Mailhot <nicolas.mailhot@xxxxxxxxxxx> 2009-01-12 16:59:41 EDT --- If spot is ok with the package I'm ok too. Full review (going a bit deeper than usual since you're also upstream) 1. Maybe change the summary to "A sans-serif cartoon font" 2. No need to write about ISO-8859-1 in the description, you'll increase the coverage with time (I hope) and then get stuck with a wrong summary (please target MES-1 coverage at least) 3. Please wrap your description at 79 columns, not 60 4. There is no need to go at such lengths to make the rpmlint relative warning go away http://fedoraproject.org/wiki/Shipping_fonts_in_Fedora_(FAQ)#rpmlint_complains_of_absolute_symbolic_links.21 5. please make your package name match the font name (if it is Serafettin Cartoon, name the package serafettin-cartoon-fonts) ✪✪✪✪✪ 6. consider using the .txt extension for your documentation files, that makes GUI users much happier 7. It's a good idea to add a fontlog.txt that traces the font history 8. your package says it's at 0.4 but the font files say they're at 0.3. Don't do that (a good trick is to have a version variable in your makefile and change the generated font version to this variable at build time, cf dejavu and liberation) 9. please ask on the fontforge mailing list how to change the FStype so the restricted font warning goes away. Most people will take it as the font not being really gpl 10. Please specify the GPL version(s) in the font metadata (v1, v2, v2+, v3, etc) (and no need to write it's under the GPL twice in the copyright field) 11. the subfamily/style ttf name in the main file seems wrong 12. More generally it looks like you still need to clean up your font metadata a bit more 13. rawhide fontforge complains about misplaced mu in the font files 14. should probably not be classified as a sans-serif font in the fontconfig file, but as a cursive or decorative one (your OS/2 metadata says it's decorative, but fontforge classifies Comic Sans MS as cursive=script in OS/2 speak) 15. please look at the substitution-font-template template in fontpackages-devel to tell fontconfig to use your font instead of TSCu_Comic when a document requests TSCu_Comic and it's not installed on-system 16. a font with Tamil glyphs should use 65 fontconfig prio at least 17. you can use http://www.unicode.org/charts/PDF/U0B80.pdf to place the tamil glyphs on the right unicode points, it would be a shame to lose them 18. adding some info in the readme on what a Serafettin is would be nice for us not in the know Apart from the package name there's nothing blocking or that can't be handled upstream, so I'll tentatively approve the package. But please change its name before filling in the CVS request ⬬⬬⬬ APPROVED ⬬⬬⬬ You can now continue from http://fedoraproject.org/wiki/Font_package_lifecycle#3.a Reassigning the request, my part is now done -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review