Bug 184291 - Review Request: gnokii - a mobile telephone manager
Review Request: gnokii - a mobile telephone manager
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ville Skyttä
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-03-07 15:51 EST by Linus Walleij
Modified: 2007-11-30 17:11 EST (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-04-03 14:23:00 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Linus Walleij 2006-03-07 15:51:57 EST
Spec Name or Url: http://www.df.lth.se/~triad/krad/fc/gnokii.spec
SRPM Name or Url: http://www.df.lth.se/~triad/krad/fc/gnokii-0.6.11-1.src.rpm
Description: Gnokii is a mobile phone swiss army knife with a library and
several command-line utilities for managing mobile phones of different kinds.
It can make data calls and retrieve/send SMS on several phone models. It
also aims to provide e.g. fax service.

This RPM is based off Ville Skyttä's RPM from livna, which has been cleared
to move into the Extras since the offending artwork once contained in it
is now in a separate package.
Comment 1 Ville Skyttä 2006-03-08 02:04:24 EST
As I'm intimately familiar with the package, it might be useful if someone else
would review this.  However, if nobody picks it up by Friday, I will do it.

Some initial comments anyway:

If the smsd-sql patch is the same as in my old package, the comment is
misleading.  It does not create the database because everything in the patch is
commented out; it's just for documentation purposes.

I think it's safe to drop the Obsoletes: in xgnokii.

Including xgnokii.pc in xgnokii doesn't seem right.  It should probably be moved
to gnokii-devel.  Or if not, xgnokii should have a dependency on pkgconfig.
Comment 2 Linus Walleij 2006-03-08 15:54:45 EST
OK fixed these things, also made a patch for the xgnokii.pc.in file in
line with the gnokii.pc.in patch.

Spec: http://www.df.lth.se/~triad/krad/fc/gnokii.spec
SRPM: http://www.df.lth.se/~triad/krad/fc/gnokii-0.6.11-2.src.rpm
Comment 3 Ville Skyttä 2006-03-12 11:16:18 EST
0.6.12 is out, could you update to it before a real review?
BTW a considerably smaller .bz2 tarball is available upstream.
Comment 5 Ville Skyttä 2006-03-18 16:32:26 EST
- FC5+: BR openssl-devel can be dropped
- FC5+: BR libXt-devel libXpm-devel needed
- standard /usr/lib64 rpath on x86_64 could be avoided eg. like (in %prep):
  sed -i -e 's|/lib /usr/lib\b|/%{_lib} %{_libdir}|' configure
- I don't see why -devel would need Requires: xgnokii, my guess is that
  xgnokii.pc exists only so that the upstream artwork package can query xgnokii's 
  prefix or something using pkgconfig, so leaving the dependency out should be ok
- please convert to UTF-8: ChangeLog CREDITS README COPYRIGHT smsd/ChangeLog
  xgnokii/ChangeLog

Other than those, looks good to me.
Comment 6 Linus Walleij 2006-03-30 14:39:18 EST
Sorry for late answer...

Ville do you know a package example that convert a few files on-the-fly
to UTF-8? I could reinvent the wheel but better not if someone else has
already...
Comment 7 Rex Dieter 2006-03-30 14:42:39 EST
$ iconv --help
Usage: iconv [OPTION...] [FILE...]
Convert encoding of given files from one encoding to another.
 Input/Output format specification:
  -f, --from-code=NAME       encoding of original text
  -t, --to-code=NAME         encoding for output
 Information:
  -l, --list                 list all known coded character sets
...
Comment 8 Linus Walleij 2006-03-31 17:20:19 EST
This package should fix the remaining issues:
Spec: http://www.df.lth.se/~triad/krad/fc/gnokii.spec
SRPM: http://www.df.lth.se/~triad/krad/fc/gnokii-0.6.12-2.src.rpm
Comment 9 Ville Skyttä 2006-04-01 01:40:41 EST
Looks good, but a couple of little things:

Please move the iconv invocations to %prep like other source modifications.  In
%install they're possibly done multiple times with rpmbuild -bi --short-circuit. 

xgnokii.pc refers to %{prefix}/share/xgnokii for xgnokii's data files, maybe it
would be a good idea to have the xgnokii package create/own that dir.

These can be changed after the import and before the first build, approved.
Comment 10 Ville Skyttä 2006-04-01 02:09:00 EST
(In reply to comment #9)
> xgnokii.pc refers to %{prefix}/share/xgnokii

Er, that's ${prefix}/share/xgnokii in pkgconfig terms and
%{_prefix}/share/xgnokii in specfile terms.
Comment 11 Linus Walleij 2006-04-03 14:23:00 EDT
OK built and released for FC-4, FC-5 and devel.
Thanks for your time, Ville, but I hope you will
keep an eye on the package in the future too!

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