Bug 176096
Summary: | Review Request: gentium-fonts: SIL Gentium fonts | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Roozbeh Pournader <roozbeh> |
Component: | Package Review | Assignee: | John Mahowald <jpmahowald> |
Status: | CLOSED NEXTRELEASE | QA Contact: | David Lawrence <dkl> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-package-review, mpeters |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2005-12-22 17:15:46 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 163779, 482985 |
Description
Roozbeh Pournader
2005-12-19 12:42:34 UTC
Someone just raised the issue on the IRC that the full URL of the source file is not mentioned in the spec file. The reason is that the full URL is utterly ugly. It's this: http://scripts.sil.org/cms/scripts/render_download.php?site_id=nrsi&format=file&media_id=Gentium_102_L_tar&filename=%2Fttf-sil-gentium_1.0.2.tar.gz - rpmlint doesn't like the license, but their FAQ states it can be packaged. http://scripts.sil.org/OFL - package meets naming guidelines, somewhat like urw-fonts - package meets packaging guidelines - license text in %doc - spec file legible, in am. english - source matches upstream (you may want to leave a comment about where to find the source) - package compiles on FC4 i386 - no missing BR - no unnecessary BR - no locales - not relocatable - owns all directories that it creates - no duplicate files - permissions ok - %clean ok - macro use consistent - code, not content - no need for -docs - nothing in %doc affects runtime - no need for .desktop file Also similar to bitstream-vera-fonts spec file. Installed package, fonts appeared in char map and OpenOffice.org, gone upon package removal. Looks good to me. However, you still need a sponsor. Perhaps we can find one of them interested interested in fonts. I would make two recommendations - 1) ass the following: Obsoletes: fonts-sil-gentium Provides: fonts-sil-gentium Reason: upstream provides an rpm with that name. It's up to you, but if upstream provides an rpm then I think it generally best to make sure that the Fedora rpm provides the same name. 2) I *think* you need to change if [ -x %{_bindir}/fc-cache ]; then %{_bindir}/fc-cache %{_datadir}/fonts fi to if [ -x %{_bindir}/fc-cache ]; then %{_bindir}/fc-cache %{_datadir}/fonts/gentium fi I seem to remember the fonts.cache-2 file not being properly created in FC5 when fc-cache is run on the %{_datadir}/fonts instead of the directory containing them. 3) I personally would get rid of %define fontdir %{_datadir}/fonts/gentium Just use %{_datadir}/fonts/gentium (In reply to comment #3) > I would make two recommendations - > > 1) ass the following: I'm so sorry. That's suppose to add the following :-/ (In reply to comment #2) > - source matches upstream (you may want to leave a comment about where to find the source) Done. (In reply to comment #3) > 1) add the following: > > Obsoletes: fonts-sil-gentium > Provides: fonts-sil-gentium Done. > I *think* you need to change > > if [ -x %{_bindir}/fc-cache ]; then > %{_bindir}/fc-cache %{_datadir}/fonts > fi > > to > > if [ -x %{_bindir}/fc-cache ]; then > %{_bindir}/fc-cache %{_datadir}/fonts/gentium > fi But that will fail to register the gentium directory in %{_datadir}/fonts/fonts.cache-1, won't it? > 3) I personally would get rid of > > %define fontdir %{_datadir}/fonts/gentium > > Just use %{_datadir}/fonts/gentium I prefer to stay with it, to minimize possible mistakes when in a later version one mistakenly writes "genitum". New package is posted at: Spec Url: http://guava.farsiweb.info/~roozbeh/gentium-fonts.spec SRPM Url: http://guava.farsiweb.info/~roozbeh/gentium-fonts-1.02-2.src.rpm BTW, Daniel Veillard has now sponsored me for cvsextras access, so I just need the package itself to be accepted. I forgot to post the ChangeLog: %changelog * Wed Dec 21 2005 Roozbeh Porunader <roozbeh> 1.02-2 - Added comment to Source0 about where to get the file - Added Provides and Obsoletes for upsteam RPM name (In reply to comment #5) > > But that will fail to register the gentium directory in > %{_datadir}/fonts/fonts.cache-1, won't it? It doesn't seem to be an issue anymore, but it doesn't specifically need to be registered in %{_datadir}/fonts/fonts.cache-{1,2} in order for the fonts to work. However - I just tested your package, and fc-cache on devel does in fact create the proper file in devel branch now when used on a parent directory - so the spec file is fine. > > > 3) I personally would get rid of > > > > %define fontdir %{_datadir}/fonts/gentium > > > > Just use %{_datadir}/fonts/gentium > > I prefer to stay with it, to minimize possible mistakes when in a later version > one mistakenly writes "genitum". That's fine. I guess it's a matter of taste. -=- This package is listed as assigned and in FE-Review. But it's assigned to default gdk Is that a mistake? -=- We should request a bug with rpmlint to get the license added to rpmlint since I would like to see othe SIL fonts with that license (when released) added to extras as well. I'll do that later tonight. Since I've already got an sponsor, it seems that a normal review should work. John, would you ACCEPT? I believe the first package must be reviewed by the person that sponsors you. http://fedoraproject.org/wiki/Extras/Contributors (Step 7). (In reply to comment #9) > I believe the first package must be reviewed by the person that sponsors you. > > http://fedoraproject.org/wiki/Extras/Contributors > (Step 7). I've seen the page, but neither Daniel nor Seth Vidal thought that would be necessary anymore. APPROVED. |