Fedora Merge Review: bitstream-vera-fonts http://cvs.fedora.redhat.com/viewcvs/devel/bitstream-vera-fonts/ Initial Owner: besfahbo
Is this package still necessary? I mean, doesn't dejavu-lgc-fonts deprecate this?
(In reply to comment #1) > Is this package still necessary? I mean, doesn't dejavu-lgc-fonts deprecate this? So it can live in Extras? :) Many people still use Bitstream Vera because DejaVu is poorly hinted.
Generlly fine. Partial review: * Consider using %{?dist} in Release (mostly useful for upgrades, as different distro versions tend to need to have different scriptlets always). * rpmlint output: E: bitstream-vera-fonts description-line-too-long the licensing FAQ in /usr/share/doc/bitstream-vera-fonts-1.10/COPYRIGHT.TXT or the W: bitstream-vera-fonts invalid-license Redistributable, with restrictions Both should be fixed. * Use "-p" option to install in %install * This: %dir %{fontdir} %{fontdir}/*.ttf could be replaced with this: %{fontdir} * Uses /usr/share/fonts without owning it or depending on a package that does (fontconfig).
(In reply to comment #2) > Many people still use Bitstream Vera because DejaVu is poorly hinted. Just for record, this seems to be an urban legend -- just asked on #dejavu and their comment was that this is nonsense, because all hinting was copied from (and some more fixed) from Vera.
I've fiexed the license and shortened the description line.
I'm a little confused. Who is the initial owner and who is doing the review? :-) Seems like Behdad is the initial owner and Matthias the reviewer, but then shouldn't the bug be assigned to Matthias and Behdad the one doing the changes on the package? And be ASSIGNED instead of NEW? If you're co-maintainers and need an "external" reviewer, I volunteer ;-)
Does this still need to be reviewed?
(In reply to comment #7) > Does this still need to be reviewed? Not if you have something to say.
Moving this back to nobody to see if someone actually wants to pick this review up...
Ok, since this is languishing in neither-land I'll do a formal review now (seems I've spent the week reviewing font packages anyway)
Formal review: OK | MUST: rpmlint must be run on every package… rpmlint *rpm 2 packages and 0 specfiles checked; 0 errors, 0 warnings. OK | MUST: The package must be named according to the Package… OK | MUST: The spec file name must match the base package… NOK | MUST: The package must meet the Packaging Guidelines… There's nothing terribly wrong with the package, but our font guidelines have evolved a little since it was created, and it's time to re-sync it with the fonts spec template in rpmdevtools. (since it's unlikely a new upstream version will ever cause a package refresh) I like to see a minimal diff when I compare a font spec to the current template in meld. OK | MUST: The package must be licensed with a Fedora approved… OK | MUST: The License field in the package spec file must… OK | MUST: If (and only if) the source package includes the… OK | MUST: The spec file must be written in American English. OK | MUST: The spec file for the package MUST be legible. OK | MUST: The sources used to build the package must match… OK | MUST: The package must successfully compile and build… N/A | MUST: If the package does not successfully compile, build OK | MUST: All build dependencies must be listed… N/A | MUST: The spec file MUST handle locales properly… N/A | MUST: Every binary RPM package which stores shared… N/A | MUST: If the package is designed to be relocatable… OK | MUST: A package must own all directories that it creates OK | MUST: A package must not contain any duplicate files OK | MUST: Permissions on files must be set properly. OK | MUST: Each package must have a %clean section OK | MUST: Each package must consistently use macros OK | MUST: The package must contain code, or permissable N/A | MUST: Large documentation files should go in a -doc N/A | MUST: If a package includes something as %doc… N/A | MUST: Header files must be in a -devel package. N/A | MUST: Static libraries must be in a -static package. N/A | MUST: Packages containing pkgconfig(.pc) files must… N/A | MUST: If a package contains library files with a suffix… N/A | MUST: In the vast majority of cases, devel packages must… N/A | MUST: Packages must NOT contain any .la libtool archives, N/A | MUST: Packages containing GUI applications must include… OK | MUST: Packages must not own files or directories already OK | MUST: At the beginning of %install, each package MUST… OK | MUST: All filenames in rpm packages must be valid UTF-8. N/A | SHOULD: If the source package does not include license - | SHOULD: The description and summary section … translations… OK | SHOULD: The package builds in mock - | SHOULD: The package builds on all supported architectures OK | SHOULD: The reviewer should test that the package… OK | SHOULD: If scriptlets are used, those scriptlets must be sane… N/A | SHOULD: Subpackages other than devel should usually require base… N/A | SHOULD: The placement of pkgconfig(.pc) files depends on… N/A | SHOULD: If the package has file dependencies outside of shortlist…
In addition please add a wiki page on this package on http://fedoraproject.org/wiki/Category:Packaged_fonts using the following template http://fedoraproject.org/wiki/Font_description_template This won't make the package any better but will give Google and our users some visibility on available fonts in Fedora, contributing to our world domination plan. That's all for this review. Reciprocating with a review of bug #261881, bug #452663, bug #454175, or even bug #454176 would be appreciated
What info should I provide Nicolas?
Just provide the info you've done what's asked in the review so I can close this bug at last :p
Nicolas, can't we either close this OBSOLETE, or you make whatever changes you like? :)
I don't think one is supposed to be both the reviewer and the modifier in a fedora merge ticket :(. Now that I've done the review part someone else needs to apply it.
Spec updated to latest guidelines as part of the F11 fonts packaging guidelines change