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.

(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.

(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.

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.u-tokyo.ac.jp> - 0.3.6-2
- Require fonts-japanese (bug 382551)

(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"
-------------------------------------

Very well.
This package is APPROVED

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

cvs done.

Rebuilt on all branches, requested on bodhi, closing.

Thank you for your review!