Bug 801073 - Review Request: kcm-fcitx - KDE Config Module for Fcitx
Review Request: kcm-fcitx - KDE Config Module for Fcitx
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Extras Quality Assurance
:
Depends On:
Blocks: kde-reviews
  Show dependency treegraph
 
Reported: 2012-03-07 10:45 EST by Liang Suilong
Modified: 2012-06-07 19:04 EDT (History)
6 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-06-07 18:59:15 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
panemade: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Liang Suilong 2012-03-07 10:45:36 EST
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 02:26:29 EDT
Please update the srpm with latest upstream tarball
Comment 3 Parag AN(पराग) 2012-05-22 06:32:09 EDT
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 10:39:20 EDT
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 10:42:16 EDT
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 12:53:40 EDT
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 13:52:53 EDT
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 14:14:28 EDT
%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 15:11:54 EDT
ah, in that case,

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

to be on the safe side. :)
Comment 10 Liang Suilong 2012-05-23 12:25:06 EDT
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 03:18:49 EDT
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 12:37:52 EDT
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 Jon Ciesla 2012-05-24 13:12:09 EDT
Git done (by process-git-requests).
Comment 14 Fedora Update System 2012-05-28 10:45:50 EDT
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 10:46:06 EDT
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 10:46:17 EDT
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 06:27:38 EDT
kcm-fcitx-0.3.3-1.fc17 has been pushed to the Fedora 17 testing repository.
Comment 18 Parag AN(पराग) 2012-06-05 23:56:48 EDT
any update here?
Comment 19 Fedora Update System 2012-06-07 18:59:15 EDT
kcm-fcitx-0.3.3-1.fc17 has been pushed to the Fedora 17 stable repository.
Comment 20 Fedora Update System 2012-06-07 18:59:28 EDT
kcm-fcitx-0.3.3-1.fc15 has been pushed to the Fedora 15 stable repository.
Comment 21 Fedora Update System 2012-06-07 19:04:31 EDT
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.