Bug 285591 - Review Request: lohit-fonts - font package for indic fonts
Summary: Review Request: lohit-fonts - font package for indic fonts
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review   
(Show other bugs)
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jens Petersen
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Keywords:
Depends On:
Blocks: 253158
TreeView+ depends on / blocked
 
Reported: 2007-09-11 04:19 UTC by Rahul Bhalerao
Modified: 2007-11-30 22:12 UTC (History)
4 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-09-24 13:34:45 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
petersen: fedora-review+
petersen: fedora-cvs+


Attachments (Terms of Use)
lohit-fonts.spec-1.patch (2.01 KB, patch)
2007-09-13 11:27 UTC, Jens Petersen
no flags Details | Diff
lohit-fonts.spec-2.patch (2.56 KB, patch)
2007-09-21 10:19 UTC, Jens Petersen
no flags Details | Diff
lohit-fonts.spec-3.patch (753 bytes, patch)
2007-09-21 16:44 UTC, Jens Petersen
no flags Details | Diff

Description Rahul Bhalerao 2007-09-11 04:19:34 UTC
Spec URL: http://rbhalera.fedorapeople.org/lohit-fonts/lohit-fonts.spec
SRPM URL: http://rbhalera.fedorapeople.org/lohit-fonts/lohit-fonts-2.1.5-1.fc8.src.rpm
Description: This package provides the Hindi, Bengali, Gujarati, Punjabi, Tamil,
Kannada, Malayalam, Oriya, Telugu TrueType/Opentype fonts.

Comment 1 Jens Petersen 2007-09-11 06:52:44 UTC
rpmlint output:

 lohit-fonts.src: E: no-changelogname-tag

need a initial change entry explaining the package is derived from fonts-indic.

 lohit-fonts.src: W: invalid-license GPL

License field needs to be updated, see:
http://fedoraproject.org/wiki/Licensing

I guess it should be GPLv2.  Unless we have some explicit font embedding exception?

Comment 2 Rahul Bhalerao 2007-09-11 10:17:11 UTC
Updated the files with the required changes. 

Comment 3 Jens Petersen 2007-09-13 11:23:11 UTC
Thanks for the update.  Please bump the release number for each new revision.

The rpmlint output is clean now. :)

Presumably for Indic we don't need fonts.dir and fonts.scale?
How about a catalogue file?
(http://fedoraproject.org/wiki/Releases/FeatureNoMoreXFS)

My review follows:

Good:
+ rpmlint clean
+ package follows upstream project name
+ package is based on fonts-indic
+ following Packaging Guidelines except as noted below
+ license is good (GPLv2) and included
+ spec file is clearly written
+ md5sum is pristine
bb9497ee772062b97ff00a1a68b17c98  fonts-indic-2.1.5.tar.gz
+ builds correctly
+ filelists are correct
+ font install scriptlets are standard

Bad:
- should not Requires fontconfig explicitly


Comment 4 Jens Petersen 2007-09-13 11:27:12 UTC
Created attachment 194541 [details]
lohit-fonts.spec-1.patch

- also should not obsolete fonts-indic packages - they will now require
lohit-fonts

and some other things I noticed.

Comment 5 Parag AN(पराग) 2007-09-13 12:13:05 UTC
from https://bugzilla.redhat.com/show_bug.cgi?id=281901#c5, retain old Source
tag instead one suggested in above patch.

Comment 6 Jens Petersen 2007-09-13 23:19:39 UTC
(In reply to comment #5)
> from https://bugzilla.redhat.com/show_bug.cgi?id=281901#c5, retain old Source
> tag instead one suggested in above patch.

Ok fair enough - I should have rebuilt before posting... ;)
(wget doesn't handle it well either - and we probably need a better
location of future releases anyway, having it on the wiki is a little
disconcerting.)

Please put the full URL as a comment above the Source line.

Comment 7 Rahul Bhalerao 2007-09-17 13:35:38 UTC
The changes have been made and the new srpm is available here:
http://rbhalera.fedorapeople.org/lohit-fonts/lohit-fonts-2.1.5-2.fc8.src.rpm


Comment 8 Jens Petersen 2007-09-21 10:17:03 UTC
- please mention the actual changes made in the changelog
- should not require fontconfig (see discussion on fedora-packaging list)

rpmlint gives:
lohit-fonts-bengali.noarch: W: no-version-in-last-changelog
lohit-fonts-gujarati.noarch: W: no-version-in-last-changelog
lohit-fonts-hindi.noarch: W: no-version-in-last-changelog
lohit-fonts-kannada.noarch: W: no-version-in-last-changelog
lohit-fonts-malayalam.noarch: W: no-version-in-last-changelog
lohit-fonts-oriya.noarch: W: no-version-in-last-changelog
lohit-fonts-punjabi.noarch: W: no-version-in-last-changelog
lohit-fonts-tamil.noarch: W: no-version-in-last-changelog
lohit-fonts-telugu.noarch: W: no-version-in-last-changelog


Comment 9 Jens Petersen 2007-09-21 10:19:34 UTC
Created attachment 201881 [details]
lohit-fonts.spec-2.patch

Here is a bunch of cleanup and fixes.

Probably this should be sufficient for this review.
Please review and make a changelog for it.

Comment 10 Jens Petersen 2007-09-21 16:44:50 UTC
Created attachment 202561 [details]
lohit-fonts.spec-3.patch

additional patch to add lohit to fontdir names

Comment 11 Parag AN(पराग) 2007-09-24 07:19:00 UTC
Spec URL: http://paragn.fedorapeople.org/lohit-fonts.spec
SRPM URL: http://paragn.fedorapeople.org/lohit-fonts-2.1.5-3.fc7.src.rpm

please review this.

Comment 12 Jens Petersen 2007-09-24 07:47:24 UTC
Here is the review:

Good:
+ rpmlint clean
+ package is named after upstream project name
+ meets packaging guidelines
+ license is gpl and included
+ spec files is clearly written
+ source is pristine
bb9497ee772062b97ff00a1a68b17c98  fonts-indic-2.1.5.tar.gz
+ noarch package builds correctly
+ owns all dirs except /usr/share/fonts (owned by fontconfig)
+ macro usage is consistent

All must items are satisfied afaict.

Package is APPROVED.

Comment 13 Parag AN(पराग) 2007-09-24 08:21:11 UTC
New Package CVS Request
=======================
Package Name: lohit-fonts
Short Description: Lohit TrueType fonts for Indic languages
Owners: rbhalera
Branches: devel
InitialCC: petersen
Cvsextras Commits: yes



Comment 14 Parag AN(पराग) 2007-09-24 08:29:50 UTC
Jens,
  Thanks for review. 

Comment 15 Jens Petersen 2007-09-24 09:10:02 UTC
cvsadmin done

Comment 16 Jens Petersen 2007-09-24 13:34:45 UTC
Thanks Parag for getting the package built.

Comment 17 Jens Petersen 2007-10-08 03:21:21 UTC
Maybe my indications on the license were not quite accurate.
According to http://fedoraproject.org/wiki/Licensing/FAQ we reach (4)
of the first question ("How do I figure out what version of the GPL/LGPL
my package is under?") and so COPYING implies GPLv2+, if I'm not mistaken.

Please update the License field to that in the next update.

We should really make the version explicit in the README file say for the next 
release of Lohit fonts though (the .ttf files themselves just say GPL).


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