Bug 615848 - Review Request: oflb-brett-fonts - A handwriting font
Review Request: oflb-brett-fonts - A handwriting font
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ankur Sinha (FranciscoD)
Fedora Extras Quality Assurance
:
Depends On:
Blocks: 477333
  Show dependency treegraph
 
Reported: 2010-07-19 01:40 EDT by Parag Nemade
Modified: 2010-07-22 01:33 EDT (History)
8 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-07-22 01:33:38 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
sanjay.ankur: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Comment 1 Paul Flo Williams 2010-07-19 06:07:05 EDT
http://fedoraproject.org/wiki/Packaging:FontsPolicy#Naming

"When foundryname, projectname or fontfamilyname contain the font or fonts affix, this affix should be dropped from them"

The name of this package should be oflb-brett-fonts.
Comment 2 Ankur Sinha (FranciscoD) 2010-07-19 06:21:19 EDT
Thanks Paul.

Parag: I'll do the review once the package name has been corrected. 

Ankur
Comment 4 Ankur Sinha (FranciscoD) 2010-07-19 07:05:26 EDT
+ OK
- NA
? ISSUE

? Package meets naming and packaging guidelines
+ Spec file matches base package name.
+ Spec has consistant macro usage.
+ Meets Packaging Guidelines.
+ License
+ License field in spec matches
+ License file included in package
+ Spec in American English
+ Spec is legible.
- Sources match upstream md5sum:

- Package needs ExcludeArch
+ BuildRequires correct
- Spec handles locales/find_lang
- Package is relocatable and has a reason to be.
+ Package has %defattr and permissions on files is good.
? Package has a correct %clean section.
? Package has correct buildroot
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
+ Package is code or permissible content.
- Doc subpackage needed/used.
+ Packages %doc files don't affect runtime.

- Headers/static libs in -devel subpackage.
- Spec has needed ldconfig in post and postun
- .pc files in -devel subpackage/requires pkgconfig
- .so files in -devel subpackage.
- -devel package Requires: %{name} = %{version}-%{release}
- .la files are removed.

- Package is a GUI app and has a .desktop file

+ Package compiles and builds on at least one arch.
+ Package has no duplicate files in %files.
+ Package doesn't own any directories other packages own.
+ Package owns all the directories it creates.
? No rpmlint output.

SHOULD Items:

+ Should build in mock.
- Should build on all supported archs
- Should function as described.
- Should have sane scriptlets.
- Should have subpackages require base package with fully versioned depend.
+ Should have dist tag
- Should package latest version
- check for outstanding bugs on package. (For core merge reviews)

Issues:

1. The ttf name is BrettFont1.1.ttf

Any particular reason why you're not using 1.1 as version and the time stamp instead?

2. rpmlint output:

oflb-brett-fonts.spec: W: no-cleaning-of-buildroot %install
oflb-brett-fonts.spec: W: no-cleaning-of-buildroot %clean
oflb-brett-fonts.spec: W: no-buildroot-tag
oflb-brett-fonts.spec: W: no-%clean-section
oflb-brett-fonts.src: W: invalid-url URL: http://openfontlibrary.org/media/files/brettalton/205 HTTP Error 404: Not Found
oflb-brett-fonts.src: W: no-cleaning-of-buildroot %install
oflb-brett-fonts.src: W: no-cleaning-of-buildroot %clean
oflb-brett-fonts.src: W: no-buildroot-tag
oflb-brett-fonts.src: W: no-%clean-section
oflb-brett-fonts.noarch: W: invalid-url URL: http://openfontlibrary.org/media/files/brettalton/205 HTTP Error 404: Not Found
oflb-brett-fonts.src: W: invalid-url URL: http://openfontlibrary.org/media/files/brettalton/205 HTTP Error 404: Not Found
oflb-brett-fonts.src: W: no-cleaning-of-buildroot %install
oflb-brett-fonts.src: W: no-cleaning-of-buildroot %clean
oflb-brett-fonts.src: W: no-buildroot-tag
oflb-brett-fonts.src: W: no-%clean-section
3 packages and 1 specfiles checked; 0 errors, 15 warnings.

Ignorable warnings. 

Please check up the clean section and build root. I also think the version would be better as 1.1 rather than the time stamp for this font. 

I'll approve it once we have the version clarified. 

Ankur
Comment 5 Parag Nemade 2010-07-19 09:01:14 EDT
The reason I kept version same as old is that, font name says its 1.1, inside font information says 1.0 and as upstream is dead, I have no way to contact upstream to clarify version info.

I will say let's import this and if needed later on we will change it to 1.0 or 1.1 version. In any way if we are removing timestamp as version, we are going to use epoch.
Comment 6 Paul Flo Williams 2010-07-19 10:31:33 EDT
Welcome to the wacky world of font metadata. To inject a bit of font archaeology:

The metadata of the original release was _really_ strange, with some fields in the name table claiming that this font was Times New Roman version 3.00, and some claiming that this was BrettFont version 1.00. For the release as part of FC8, Jon Stanley used 1.0-2 as the nvr.

In May 2008, a new version was uploaded to OFLB, with the most obvious tell-tales of its hacky origin removed. Unfortunately, the metadata still claimed this as BrettFont 1.0. In the face of this confusion, Jon started using the date of release as the version.

I agree with Parag; there isn't likely to be a future release (unless Brett removes the final few "Monotype" strings from the metadata), so continuing to treat the internal version as unreliable and going for dates makes sense.

For the record, I'm examining:

sha1sum *.ttf
9fd5585ec2b2c9d9850a5a8e9acd94b0c2d2e9f1  brettalton_-_Brett_Font.ttf
424d6ef6eaa4a71c719b51ae39ef268f6f011418  BrettFont1.1.ttf
Comment 7 Ankur Sinha (FranciscoD) 2010-07-20 14:02:12 EDT
hey Paul, 

thanks for the history lesson :)

Guess it's approved then 

XXX APPROVED XXX
Comment 8 Parag Nemade 2010-07-20 23:35:15 EDT
Thanks for the review!
Thanks Paul for your feedback.

New Package CVS Request
=======================
Package Name: oflb-brett-fonts
Short Description: A handwriting font 
Owners: pnemade
Branches: 
InitialCC: i18n-team fonts-sig
Comment 9 Kevin Fenzi 2010-07-21 01:17:00 EDT
CVS done (by process-cvs-requests.py).
Comment 10 Parag Nemade 2010-07-22 01:33:38 EDT
Thanks all. Built in rawhide.

Note You need to log in before you can comment on or make changes to this bug.