Bug 225767 - Merge Review: fonts-korean
Merge Review: fonts-korean
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Extras Quality Assurance
: i18n
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-01-31 13:39 EST by Nobody's working on this, feel free to take it
Modified: 2007-11-30 17:11 EST (History)
5 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-08-06 20:26:37 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
panemade: fedora‑review+


Attachments (Terms of Use)
some modification to SPEC (2.96 KB, patch)
2007-02-28 07:02 EST, Parag AN(पराग)
no flags Details | Diff
Final Cleaned SPEC (4.12 KB, text/x-rpm-spec)
2007-07-26 03:51 EDT, Parag AN(पराग)
no flags Details
Little changes on last attachment from Parag. (4.21 KB, application/octet-stream)
2007-07-29 11:35 EDT, Caius Chance
no flags Details
Little changes on last attachment from Parag. (4.21 KB, application/octet-stream)
2007-07-29 11:35 EDT, Caius Chance
no flags Details
SPEC with Provides & Obsoletes version specification. (4.30 KB, application/octet-stream)
2007-07-30 01:37 EDT, Caius Chance
no flags Details
Spec file. (4.86 KB, text/plain)
2007-07-31 03:22 EDT, Caius Chance
no flags Details

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 13:39:44 EST
Fedora Merge Review: fonts-korean

http://cvs.fedora.redhat.com/viewcvs/devel/fonts-korean/
Initial Owner: cchance@redhat.com
Comment 1 Parag AN(पराग) 2007-02-28 07:01:20 EST
Mock build in rawhide is sucessful. 
But build.lod showed
warning: File listed twice: /usr/share/fonts/korean/TrueType
warning: File listed twice: /usr/share/fonts/korean/TrueType/fonts.cache-1
warning: File listed twice: /usr/share/fonts/korean/TrueType/fonts.dir
warning: File listed twice: /usr/share/fonts/korean/TrueType/fonts.scale
warning: File listed twice: /usr/share/fonts/korean/misc
warning: File listed twice: /usr/share/fonts/korean/misc/encodings.dir
warning: File listed twice: /usr/share/fonts/korean/misc/fonts.cache-1
warning: File listed twice: /usr/share/fonts/korean/misc/fonts.dir


Then rpmlint repotred some errors as
On fonts-korean-1.0.11-9.1.1.src.rpm
W: fonts-korean no-url-tag
The URL tag is missing.

W: fonts-korean unversioned-explicit-obsoletes ttfonts-ko
The specfile contains an unversioned Obsoletes: token, which will match all
older, equal and newer versions of the obsoleted thing.  This may cause update
problems, restrict future package/provides naming, and may match something it
was originally not inteded to match -- make the Obsoletes versioned if
possible.

W: fonts-korean setup-not-quiet
You should use -q to have a quiet extraction of the source tarball, as this
generate useless lines of log ( for buildbot, for example )

W: fonts-korean mixed-use-of-spaces-and-tabs (spaces: line 17, tab: line 1)
The specfile mixes use of spaces and tabs for indentation, which is a
cosmetic annoyance.  Use either spaces or tabs for indentation, not both.

On fonts-korean-1.0.11-9.1.1.noarch.rpm
W: fonts-korean no-url-tag
The URL tag is missing.

E: fonts-korean obsolete-not-provided ttfonts-ko
The obsoleted package must also be provided to allow clean upgrade paths
and not to break dependencies.

W: fonts-korean non-conffile-in-etc /etc/ghostscript/FAPIcidfmap.ko
A non-executable file in your package is being installed in /etc, but is not
a configuration file. All non-executable files in /etc should be configuration
files. Mark the file as %config in the spec file.

W: fonts-korean non-conffile-in-etc /etc/ghostscript/cidfmap.ko
A non-executable file in your package is being installed in /etc, but is not
a configuration file. All non-executable files in /etc should be configuration
files. Mark the file as %config in the spec file.


I did some work for you.
Comment 2 Parag AN(पराग) 2007-02-28 07:02:30 EST
Created attachment 148910 [details]
some modification to SPEC
Comment 3 Caius Chance 2007-04-10 20:33:17 EDT
Hi Parag, what could I help here? Please feel free to let me know. - Caius.
Comment 4 Parag AN(पराग) 2007-07-26 03:51:17 EDT
Created attachment 159993 [details]
Final Cleaned SPEC

Use this SPEC file. Though this is not rpmlint silent SPEC. you need to add
versions to Obsoletes and Provides.
Everything else is good.
Comment 5 Caius Chance 2007-07-29 11:35:09 EDT
Created attachment 160193 [details]
Little changes on last attachment from Parag.

Used attachment# 159993 [details], with following changes:
- Replaced /usr/bin by macro %{_binsir} and /usr/sbin by macro %{_sbindir}.
- Minor alignments.

P.S. We still need font.alias for this package before into build system.
Comment 6 Caius Chance 2007-07-29 11:35:28 EDT
Created attachment 160194 [details]
Little changes on last attachment from Parag.

Used attachment# 159993 [details], with following changes:
- Replaced /usr/bin by macro %{_binsir} and /usr/sbin by macro %{_sbindir}.
- Minor alignments.

P.S. We still need font.alias for this package before into build system.
Comment 7 Caius Chance 2007-07-29 11:38:12 EDT
Hi Jens, would you like to have a look? Cheers, Caius.
Comment 8 Jens Petersen 2007-07-29 19:51:31 EDT
BTW I am still thinking we should rename fonts-* to <upstreamname>-fonts
for F8 - let me send out a mail about it to the fedora devel and i18n lists
for comments.  But maybe it is better to finish the reviews first before
renaming anyway.
Comment 9 Caius Chance 2007-07-29 20:27:40 EDT
(FYI, creation of fonts.alias is not necessary because there is an existing one
within archive file: baekmuk-2.1.tar.gz)

For srpm review:

http://hiro.brisbane.redhat.com/~cchance/20070729_f-k/fonts-korean-1.0.11-10.fc8.src.rpm

http://hiro.brisbane.redhat.com/~cchance/20070729_f-k/fonts-korean.spec

=====

Do you mean "baekmuk-fonts"? If so, do we need a meta-package to let user able
to install korean support in CLI by just installing font-korean package?
Comment 10 Parag AN(पराग) 2007-07-30 00:24:44 EDT
rpmlint on SRPM gave me
W: fonts-korean unversioned-explicit-obsoletes ttfonts-ko
The specfile contains an unversioned Obsoletes: token, which will match all
older, equal and newer versions of the obsoleted thing.  This may cause update
problems, restrict future package/provides naming, and may match something it
was originally not inteded to match -- make the Obsoletes versioned if
possible.

W: fonts-korean unversioned-explicit-provides ttfonts-ko
The specfile contains an unversioned Provides: token, which will match all
older, equal, and newer versions of the provided thing.  This may cause
update problems and will make versioned dependencies, obsoletions and conflicts
on the provided thing useless -- make the Provides versioned if possible.
Comment 11 Caius Chance 2007-07-30 01:37:50 EDT
Created attachment 160215 [details]
SPEC with Provides & Obsoletes version specification.
Comment 12 Jens Petersen 2007-07-30 02:28:42 EDT
> Do you mean "baekmuk-fonts"? If so, do we need a meta-package to let user able
> to install korean support in CLI by just installing font-korean package?

Actually looking at fonts-korean now, it seems it only includes baekmuk fonts
so it could probably just be a straightward rename to "baekmuk-fonts". :)
Perhaps it could provide fonts-korean if necessary or a meta package might
still be useful for pulling other common korean fonts.
Comment 13 Jens Petersen 2007-07-30 02:41:23 EDT
Please update the source and source url to
http://kldp.net/frs/?group_id=57&release_id=865

The current version is 2.2.
Comment 15 Jens Petersen 2007-07-31 01:20:56 EDT
Seems to me the version numbering is a bit strange, no?
I think it should reflect the upstream version which is 2.2 now.
Comment 16 Parag AN(पराग) 2007-07-31 02:10:06 EDT
(In reply to comment #15)
> Seems to me the version numbering is a bit strange, no?
> I think it should reflect the upstream version which is 2.2 now.

yes. it should be changed to 2.2 version.
else package looks ok.
Comment 17 Caius Chance 2007-07-31 02:14:43 EDT
1.0.11-10 is fonts-korean's version.
2.2 is baekmuk's version.

IMHO, if one day fonts-korean be renamed to baekmuk-fonts, then we could change
to 2.2 then. BTW FYI, a renaming will redirect all the previous fonts-korean
history to baekmuk-fonts, and it might be lots of work to do if there is a meta
package fonts-korean reintroduced in future.
Comment 18 Jens Petersen 2007-07-31 02:27:31 EDT
(In reply to comment #17)
> 1.0.11-10 is fonts-korean's version.
> 2.2 is baekmuk's version.

Yes, but fonts-korean is just a collection of baekmuk fonts, so
it should follow the upstream version number.

> renaming will redirect all the previous fonts-korean
> history to baekmuk-fonts, and it might be lots of work to do if there is a 
> meta package fonts-korean reintroduced in future.

Yes it will be some work to rename the package
but I think it is a meaningful and correct change to make.
change.
Comment 20 Parag AN(पराग) 2007-07-31 02:58:29 EDT
Looks ok to me and ready for review.
will post formal review now.
Comment 21 Parag AN(पराग) 2007-07-31 03:02:28 EDT
can't find source url tag
Comment 22 Caius Chance 2007-07-31 03:22:16 EDT
Created attachment 160296 [details]
Spec file.

Spec file with URL tag.
Comment 23 Parag AN(पराग) 2007-07-31 04:18:10 EDT
Review:
+ package builds in mock (development i386).
+ rpmlint is silent for SRPM and for RPM.
+ source files match upstream url
48c0405ffbdf45e1311a978b65b47f4c  baekmuk-bdf-2.2.tar.gz
a6f4349179cbe3557641782cefba4d70  baekmuk-ttf-2.2.tar.gz
e8f61c1ad118bc0f22e9373864866c60  cidfmap.ko
53056f07c8eefbcc63a3f14cb3faa724  FAPIcidfmap.ko
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ build root is correct.
+ license is open source-compatible.
+ License text is included in package.
+ %doc is present.
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ Package contains content.
+ no headers or static libraries.
+ no .pc file present.
+ no -devel subpackage
+ no .la files.
+ no translations are available
+ Does owns the directories it creates.
+ fonts scriptlets present.
+ no duplicates in %files.
+ file permissions are appropriate.
+ Not a GUI App.
APPROVED.
Comment 24 Caius Chance 2007-08-06 20:26:37 EDT
Thanks a million, Parag!

Built in rawhide.
CLOSED RAWHIDE.
Comment 25 Jens Petersen 2007-08-17 03:33:08 EDT
The Source1 and Source2 fields should list the full url to the source:

Source1:  http://kldp.net/frs/download.php/1428/baekmuk-bdf-2.2.tar.gz
Source2:  http://kldp.net/frs/download.php/1429/baekmuk-ttf-2.2.tar.gz
Comment 26 Caius Chance 2007-08-21 03:22:07 EDT
Updated .spec file according to comment# 25.

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