Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 801073 - Review Request: kcm-fcitx - KDE Config Module for Fcitx
Summary: Review Request: kcm-fcitx - KDE Config Module for Fcitx
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Parag AN(पराग)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: kde-reviews
TreeView+ depends on / blocked
 
Reported: 2012-03-07 15:45 UTC by Liang Suilong
Modified: 2012-06-07 23:04 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2012-06-07 22:59:15 UTC
Type: ---
Embargoed:
panemade: fedora-review+
gwync: fedora-cvs+


Attachments (Terms of Use)

Description Liang Suilong 2012-03-07 15:45:36 UTC
SPEC: http://liangsuilong.fedorapeople.org/fcitx/kcm-fcitx.spec
SRPM: http://liangsuilong.fedorapeople.org/fcitx/kcm-fcitx-0.3.0-1.fc16.src.rpm
Description:
Kcm-fcitx is a System Settings module to manage service menus.
The module can install and remove service menus from local 
files and the internet (KNewStuff).
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=3864100

Comment 1 Parag AN(पराग) 2012-05-15 06:26:29 UTC
Please update the srpm with latest upstream tarball

Comment 3 Parag AN(पराग) 2012-05-22 10:32:09 UTC
Review:
+ koji build -> http://koji.fedoraproject.org/koji/taskinfo?taskID=4091556

+ rpmlint on rpms gave
kcm-fcitx.x86_64: E: incorrect-fsf-address /usr/share/doc/kcm-fcitx-0.3.3/COPYING
kcm-fcitx.x86_64: E: zero-length /usr/share/doc/kcm-fcitx-0.3.3/README
2 packages and 0 specfiles checked; 2 errors, 0 warnings.

+ Source verified with upstream as (sha1sum)
83a431d51df5cb2fa049e8e59ab5ccc3685adde0  kcm-fcitx-0.3.3.tar.xz
83a431d51df5cb2fa049e8e59ab5ccc3685adde0  ../SOURCES/kcm-fcitx-0.3.3.tar.xz

+ Package kcm-fcitx-0.3.3-1.fc18.x86_64 =>
 Provides: kcm-fcitx = 0.3.3-1.fc18 kcm-fcitx(x86-64) = 0.3.3-1.fc18 kcm_fcitx.so()(64bit)
 Requires: libQtCore.so.4()(64bit) libQtDBus.so.4()(64bit) libQtGui.so.4()(64bit) libc.so.6()(64bit) libc.so.6(GLIBC_2.14)(64bit) libc.so.6(GLIBC_2.2.5)(64bit) libc.so.6(GLIBC_2.3.4)(64bit) libc.so.6(GLIBC_2.8)(64bit) libfcitx-config.so.4()(64bit) libfcitx-core.so.0()(64bit) libfcitx-utils.so.0()(64bit) libkdecore.so.5()(64bit) libkdeui.so.5()(64bit) libkio.so.5()(64bit) libknewstuff3.so.4()(64bit) libstdc++.so.6()(64bit) libstdc++.so.6(CXXABI_1.3)(64bit) libstdc++.so.6(GLIBCXX_3.4)(64bit) rtld(GNU_HASH)


Suggestions:
1) Good to use commands in spec actually.
replace
%{__mkdir} -pv build
with
mkdir -pv build

2) Remove the zero-length files from installation. For this package remove README from %doc

3) I can't see MimeType key written in desktop files. There is no need of writing http://fedoraproject.org/wiki/Packaging:ScriptletSnippets#desktop-database in spec file. Please remove it.

Comment 4 Rex Dieter 2012-05-22 14:39:20 UTC
Kcm's need a dependency:

# need kcmshell4 from kde(base)-runtime at least
Requires: kdebase-runtime%{?_kde4_version: >= %{_kde4_version}}

Comment 5 Rex Dieter 2012-05-22 14:42:16 UTC
I notice too that the pkg summary/description doesn't match what's listed on
http://code.google.com/p/fcitx/

the current ones don't mention input methods at all, and upstream site doesn't mention anything about service menus or GHNS

Comment 6 Liang Suilong 2012-05-22 16:53:40 UTC
I updated a new spec file and a new SRPM packages. Please continue to review it. 

Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=4094928

SPEC: http://liangsuilong.fedorapeople.org/fcitx/kcm-fcitx.spec
SRPM: http://liangsuilong.fedorapeople.org/fcitx/kcm-fcitx-0.3.3-1.fc16.src.rpm

Comment 7 Rex Dieter 2012-05-22 17:52:53 UTC
Several suggestions:

1.  replace
cat << EOF > %{name}.lang 
%lang(zh) /usr/share/locale/zh_TW/LC_MESSAGES/kcm_fcitx.mo
%lang(zh) /usr/share/locale/zh_CN/LC_MESSAGES/kcm_fcitx.mo
EOF

with
%find_lang %{name}


2.  in %build

you have
pushd build

but no matching
popd


3.  (cosmetics, but shorter) in %install, replace
pushd build
make install DESTDIR=$RPM_BUILD_ROOT
popd

with
make install  DESTDIR=$RPM_BUILD_ROOT -C build

Comment 8 Kevin Kofler 2012-05-22 18:14:28 UTC
%find_lang %{name} is not going to work because %{name} uses a '-' rather than an '_'. You'll have to use %find_lang kcm_fcitx. (Or maybe the package should be called kcm_fcitx? But the tarball doesn't match that. We have this mess about kcm-* vs. kcm_* because of Debian's restrictive package name policies which ban underscores.)

Comment 9 Rex Dieter 2012-05-22 19:11:54 UTC
ah, in that case,

find_lang %{name} --all-name --with-kde

to be on the safe side. :)

Comment 10 Liang Suilong 2012-05-23 16:25:06 UTC
I have updated the latest SPEC file and SRPM file. 

SPEC: http://liangsuilong.fedorapeople.org/fcitx/kcm-fcitx.spec
SRPM: http://liangsuilong.fedorapeople.org/fcitx/kcm-fcitx-0.3.3-1.fc16.src.rpm
Koji: http://koji.fedoraproject.org/koji/taskinfo?taskID=4096647

(In reply to comment #7)
> Several suggestions:
> 
> 1.  replace
> cat << EOF > %{name}.lang 
> %lang(zh) /usr/share/locale/zh_TW/LC_MESSAGES/kcm_fcitx.mo
> %lang(zh) /usr/share/locale/zh_CN/LC_MESSAGES/kcm_fcitx.mo
> EOF
> 
> with
> %find_lang %{name}
> 

Fixed

> 
> 2.  in %build
> 
> you have
> pushd build
> 
> but no matching
> popd
> 

Fixed

> 
> 3.  (cosmetics, but shorter) in %install, replace
> pushd build
> make install DESTDIR=$RPM_BUILD_ROOT
> popd
> 
> with
> make install  DESTDIR=$RPM_BUILD_ROOT -C build


Fixed

(In reply to comment #9)
> ah, in that case,
> 
> find_lang %{name} --all-name --with-kde
> 
> to be on the safe side. :)

Comment 11 Parag AN(पराग) 2012-05-24 07:18:49 UTC
Looks fine now. Thanks others also for your inputs. 

Just remove README from %doc as its zero-length file.

APPROVED.

Comment 12 Liang Suilong 2012-05-24 16:37:52 UTC
New Package SCM Request
=======================
Package Name: kcm-fcitx
Short Description: KDE Config Module for Fcitx
Owners: liangsuilong
Branches: f15 f16 f17 el6
InitialCC: i18n-team

Comment 13 Gwyn Ciesla 2012-05-24 17:12:09 UTC
Git done (by process-git-requests).

Comment 14 Fedora Update System 2012-05-28 14:45:50 UTC
kcm-fcitx-0.3.3-1.fc16 has been submitted as an update for Fedora 16.
https://admin.fedoraproject.org/updates/kcm-fcitx-0.3.3-1.fc16

Comment 15 Fedora Update System 2012-05-28 14:46:06 UTC
kcm-fcitx-0.3.3-1.fc15 has been submitted as an update for Fedora 15.
https://admin.fedoraproject.org/updates/kcm-fcitx-0.3.3-1.fc15

Comment 16 Fedora Update System 2012-05-28 14:46:17 UTC
kcm-fcitx-0.3.3-1.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/kcm-fcitx-0.3.3-1.fc17

Comment 17 Fedora Update System 2012-05-29 10:27:38 UTC
kcm-fcitx-0.3.3-1.fc17 has been pushed to the Fedora 17 testing repository.

Comment 18 Parag AN(पराग) 2012-06-06 03:56:48 UTC
any update here?

Comment 19 Fedora Update System 2012-06-07 22:59:15 UTC
kcm-fcitx-0.3.3-1.fc17 has been pushed to the Fedora 17 stable repository.

Comment 20 Fedora Update System 2012-06-07 22:59:28 UTC
kcm-fcitx-0.3.3-1.fc15 has been pushed to the Fedora 15 stable repository.

Comment 21 Fedora Update System 2012-06-07 23:04:31 UTC
kcm-fcitx-0.3.3-1.fc16 has been pushed to the Fedora 16 stable repository.


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