This service will be undergoing maintenance at 20:00 UTC, 2017-04-03. It is expected to last about 30 minutes
Bug 382551 - Review Request: ebview - EPWING CD-ROM dictionary viewer
Review Request: ebview - EPWING CD-ROM dictionary viewer
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Alexander Kahl
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-11-14 09:48 EST by Mamoru TASAKA
Modified: 2007-11-30 17:12 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-11-16 01:42:50 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
e-user: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Mamoru TASAKA 2007-11-14 09:48:57 EST
Spec URL: http://mtasaka.fedorapeople.org/Review_request/ebview/ebview.spec
SRPM URL: http://mtasaka.fedorapeople.org/Review_request/ebview/ebview-0.3.6-1.fc8.src.rpm
Description: 
EBView is a EPWING dictionary browser.

koji build is successful (on dist-f9).
http://koji.fedoraproject.org/koji/taskinfo?taskID=241335
Comment 1 Alexander Kahl 2007-11-14 13:13:55 EST
Package Review

* rpmlint remains silent
* The package is named according to the Package Naming Guidelines
* The spec file name matches the base package
* Fedora approved license: GPLv2+
* License field in the package spec file matches the actual license
* COPYING included
* The spec file is written in American English
* The spec file is legible
* The sources used to build the package matches the upstream source:
SHA1 47b63fb2f265c83cc5bf7aeb92f96d0e1cc7f9af
* Builds on f8 x86_64
* BuildRequires look sane
* Proper locale handling
* No libraries
* Package owns all directories it creates
* No duplicates in %files
* File permissions are sane
* %clean starts with build root clean
* Consistent use of macros
* The package contains code
* Documentation not large, no -doc subpackage needed
* No header files
* No static libs
* No pkgconfig files
* No library files
* Desktop file handled properly (BuildRequires, %install, vendor id)
* Package does not own files or directories already owned by other packages
* %install starts with build root clean
* All filenames are valid UTF-8

Additional checks:
* Latest version is being packaged: 0.3.6
* Dist tag is present
* Group tag is valid
* Build root is correct:
%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)
* Scriptlets look sane
* Builds in mock (dist-f9)

Only three questionable items:
- /usr/share/ebview/about.jp is not UTF-8 but ISO-8859 encoded, is this
necessary for the runtime?

- ebview.spec:68: --with-eb-conf=%{_libdir}/eb.conf
Neither does the package provide such a file nor does it belong there, according
to ./configure --help output:
"  --with-eb-conf=FILE     eb.conf file is FILE [default=SYSCONFDIR/eb.conf]"
Configuration files should reside in %{_sysconfdir} and (if provided) handled
properly in %files

- On startup, pango warns
(ebview:9142): Pango-WARNING **: couldn't load font "Sazanami Mincho 12",
falling back to "Sans 12", expect ugly output.
Since your scriptlet (re)defines the default font in src/preference.c to
Sazanami, you should add a Require for this font.
If the font is not available in Fedora yet, could you create a package for it?

After clarification of these items I'm going to approve the package.
Comment 2 Mamoru TASAKA 2007-11-15 05:42:57 EST
(In reply to comment #1) 
> Only three questionable items:
> - /usr/share/ebview/about.jp is not UTF-8 but ISO-8859 encoded, is this
> necessary for the runtime?
  - This is encoded with EUC-JP (, which is very common on Japanese
    Unix system) and this file is actually needed.
> 
> - ebview.spec:68: --with-eb-conf=%{_libdir}/eb.conf
  %_libdir/eb.conf is included in eb-devel, not in this package.

> - On startup, pango warns
> (ebview:9142): Pango-WARNING **: couldn't load font "Sazanami Mincho 12",
> falling back to "Sans 12", expect ugly output.
> Since your scriptlet (re)defines the default font in src/preference.c to
> Sazanami, you should add a Require for this font.
  Sazanami is in default Japanese desktop environ (from comps.xml).
  Anyway this package requires Japanese fonts (this package is for
  Japanese people), and even if Sazanami is not installed, this
  package should work is one Japanese font (in Fedora) is installed.
Comment 3 Alexander Kahl 2007-11-15 06:28:22 EST
(In reply to comment #2)
> (In reply to comment #1) 
> > Only three questionable items:
> > - /usr/share/ebview/about.jp is not UTF-8 but ISO-8859 encoded, is this
> > necessary for the runtime?
>   - This is encoded with EUC-JP (, which is very common on Japanese
>     Unix system) and this file is actually needed.
OK

> > - ebview.spec:68: --with-eb-conf=%{_libdir}/eb.conf
>   %_libdir/eb.conf is included in eb-devel, not in this package.
I see, just checked eb-devel's spec file. I wonder how eb.conf could possibly
belong into %{_libdir}, but that issue is not part of this review.
 
> > - On startup, pango warns
> > (ebview:9142): Pango-WARNING **: couldn't load font "Sazanami Mincho 12",
> > falling back to "Sans 12", expect ugly output.
> > Since your scriptlet (re)defines the default font in src/preference.c to
> > Sazanami, you should add a Require for this font.
>   Sazanami is in default Japanese desktop environ (from comps.xml).
>   Anyway this package requires Japanese fonts (this package is for
>   Japanese people), and even if Sazanami is not installed, this
>   package should work is one Japanese font (in Fedora) is installed.
Could you please add
Requires: sazanami-fonts-mincho
anyway so even on non-Japanese installations all characters are displayed
properly? I don't have any Japanese font installed and many characters are
unavailable.
Comment 4 Mamoru TASAKA 2007-11-15 06:44:49 EST
Well, instead I added "Requires: fonts-japanese".

http://mtasaka.fedorapeople.org/Review_request/ebview/ebview.spec
http://mtasaka.fedorapeople.org/Review_request/ebview/ebview-0.3.6-2.fc8.src.rpm

----------------------------------------------------
* Thu Nov 15 2007 Mamoru Tasaka <mtasaka@ioa.s.u-tokyo.ac.jp> - 0.3.6-2
- Require fonts-japanese (bug 382551)
Comment 5 Mamoru TASAKA 2007-11-15 07:05:20 EST
(In reply to comment #3)
> > > - ebview.spec:68: --with-eb-conf=%{_libdir}/eb.conf
> >   %_libdir/eb.conf is included in eb-devel, not in this package.
> I see, just checked eb-devel's spec file. I wonder how eb.conf could possibly
> belong into %{_libdir}, but that issue is not part of this review.
  I guess this is because %_libdir/eb.conf contains arch-specific
  parts.
-------------------------------------
EBCONF_EBLIBS="-L/usr/lib -leb"
-------------------------------------
Comment 6 Alexander Kahl 2007-11-15 09:05:13 EST
Very well.
This package is APPROVED
Comment 7 Mamoru TASAKA 2007-11-15 09:22:48 EST
Very thanks!
(BTW now I am checking your bmpx)

New Package CVS Request
=======================
Package Name:      ebview
Short Description: EPWING CD-ROM dictionary viewer
Owners:            mtasaka
Branches:          F-8 F-7
InitialCC: 
Cvsextras Commits: yes
Comment 8 Kevin Fenzi 2007-11-15 12:21:10 EST
cvs done.
Comment 9 Mamoru TASAKA 2007-11-16 01:42:50 EST
Rebuilt on all branches, requested on bodhi, closing.

Thank you for your review!

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