Bug 682130

Summary: Review Request: ibus-indic-table - Indic Table engine for IBus platform
Product: [Fedora] Fedora Reporter: Naveen Kumar <nkumar>
Component: Package ReviewAssignee: Parag AN(पराग) <panemade>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: fedora-package-review, i18n-bugs, notting, panemade, psatpute
Target Milestone: ---Flags: panemade: fedora-review+
j: fedora-cvs+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-03-10 06:47:26 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Comment 2 Parag AN(पराग) 2011-03-07 10:36:33 UTC
I see that you have a new tarball updated with no date suffix.

koji build => http://koji.fedoraproject.org/koji/taskinfo?taskID=2890131


Please add some detailed changelog to understand what got changed from older submission to this new srpm. Re-upload the same srpm with added changelog.

Comment 3 Naveen Kumar 2011-03-08 06:10:12 UTC
updated the same spec and SRPM

koji scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2893142

Comment 4 Parag AN(पराग) 2011-03-08 07:34:59 UTC
Suggestions:
1) split changelog lines to 80 character per line
2) versions are not needed in Requires: and BuildRequires:
3) Include ChangeLog file as %doc but looks like ChangeLog file carries contents from ibus-table ChangeLog file.
4) Good if upstream will add information related to ibus-indic-table project in %doc files and just mention in README that this project is a fork from ibus-table project.
5) I think you don't need file pinyin_table.txt.bz2 installed.
6) I also see template.txt should be modified to have unique uuid.

Overall, it will be good if you will release a new tarball and package it as an update.

Comment 6 Parag AN(पराग) 2011-03-09 04:20:12 UTC
Suggestions:
1) --disable-static is not needed to %configure
2) preserve the timestamps of installed files.
make DESTDIR=${RPM_BUILD_ROOT} NO_INDEX=true \
install pkgconfigdir=%{_datadir}/pkgconfig \
 INSTALL="install -p"

APPROVED.

Comment 7 Pravin Satpute 2011-03-09 06:57:24 UTC
after install package i found 

1) /usr/share/ibus-indic-table/indic-tables

as well

2) /usr/share/ibus-indic-table/tables

i think we only need second

Comment 8 Naveen Kumar 2011-03-09 08:58:47 UTC
(In reply to comment #6)
> Suggestions:
> 1) --disable-static is not needed to %configure
> 2) preserve the timestamps of installed files.
> make DESTDIR=${RPM_BUILD_ROOT} NO_INDEX=true \
> install pkgconfigdir=%{_datadir}/pkgconfig \
>  INSTALL="install -p"
> 
> APPROVED.

Thanks Parag :)

Comment 9 Naveen Kumar 2011-03-09 09:24:37 UTC
New Package SCM Request
=======================
Package Name: ibus-indic-table
Short Description: Indic Table engine for IBus platform
Owners: nkumar
Branches: f14 f15
InitialCC: i18n-team

Comment 10 Jason Tibbitts 2011-03-09 14:28:47 UTC
Git done (by process-git-requests).

Comment 11 Naveen Kumar 2011-03-10 05:53:43 UTC
Thanks Jason :)