Bug 630932 - Review Request: ibus-table-others- Various tables for IBus-Table
Summary: Review Request: ibus-table-others- Various tables for IBus-Table
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Parag AN(पराग)
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
: 595551 (view as bug list)
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-09-07 12:23 UTC by Naveen Kumar
Modified: 2010-10-22 11:42 UTC (History)
6 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2010-10-22 11:42:57 UTC
Type: ---
Embargoed:
panemade: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Naveen Kumar 2010-09-07 12:23:23 UTC
Spec URL: http://nkumar.fedorapeople.org/ibus-table-others/ibus-table-others.spec
SRPM URL: http://nkumar.fedorapeople.org/ibus-table-others/ibus-table-others-1.3.0.20100907-1.fc13.src.rpm
Description: The package contains various IBus-Tables which include languages of Latin-America, Europe, Southeast Asia, as well as math and other symbols

Hi! I have just finished packaging ibus-table-others, and I would appreciate a review so that the package can get into fedora.

Comment 1 Jens Petersen 2010-09-09 05:48:35 UTC
*** Bug 595551 has been marked as a duplicate of this bug. ***

Comment 2 Chen Lei 2010-09-14 17:24:13 UTC
I'd like to see this package to be splitted into more subpackages.

Comment 3 Naveen Kumar 2010-09-20 12:10:59 UTC
(In reply to comment #2)
> I'd like to see this package to be splitted into more subpackages.

yep, I will do that, just give me some time.

Comment 5 Chen Lei 2010-09-30 09:12:54 UTC
1. Since you already split ibus-table-others into several subpackages, so  Obsoletes and Provides fields are useless.

2. %package ibus-table-code ->
%package -n ibus-table-code

%files ibus-table-code
->
%files -n ibus-table-code


3.
%files
%defattr(-,root,root,-)
%doc AUTHORS COPYING README

Those line can be removed.


4.

I suggest to add a blank line between Summary: and %description.


5.

I suggest to use rm and make to macros %__make and %__rm


6.
Requires:         ibus-table >= 1.2.0.20090912
You should move this line to every subpackage separately.

Comment 6 Naveen Kumar 2010-09-30 13:07:30 UTC
(In reply to comment #5)
> 1. Since you already split ibus-table-others into several subpackages, so 
> Obsoletes and Provides fields are useless.

I have not removed Obsoletes field from the subpackages, since there were packages with the same name before kaio joined them in ibus-table-others. The tables are same.


Updated Spec URL:
http://nkumar.fedorapeople.org/ibus-table-others/1.3.0.20100907-3/ibus-table-others.spec

Updated SRPM URL:
http://nkumar.fedorapeople.org/ibus-table-others/1.3.0.20100907-3/ibus-table-others-1.3.0.20100907-3.fc13.src.rpm

Koji Scratch Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2503543

Comment 7 Naveen Kumar 2010-10-06 10:14:03 UTC
Updated Spec URL: http://nkumar.fedorapeople.org/ibus-table-others/1.3.0.20100907-4/ibus-table-others.spec


Updated SRPM URL: http://nkumar.fedorapeople.org/ibus-table-others/1.3.0.20100907-4/ibus-table-others-1.3.0.20100907-4.fc13.src.rpm

Koji Scratch Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2517280

* Wed Oct 6 2010 Naveen Kumar <nkumar> - 1.3.0.20100907-4
- delete all Obsoletes fields in spec
- removed BuildRoot as specified in new guidelines

Comment 8 Naveen Kumar 2010-10-12 10:26:43 UTC
@Chen Lei If you have time, can you please finish the next round of review, so that if there are no errors, we can move it to fedora. Waiting for your reply...

Comment 9 Parag AN(पराग) 2010-10-19 10:37:03 UTC
+ Koji build=> http://koji.fedoraproject.org/koji/taskinfo?taskID=2542593
+ rpmlint output
ibus-table-cyrillic.noarch: W: spelling-error %description -l en_US rustrad -> rust rad, rust-rad, frustrated
ibus-table-cyrillic.noarch: W: spelling-error %description -l en_US yawerty -> yawmeter, QWERTY, qwerty
ibus-table-latin.noarch: W: spelling-error %description -l en_US ipa -> IPA, pa, nipa
ibus-table-latin.noarch: W: spelling-error %description -l en_US sampa -> sampan, salpa, pampa
ibus-table-others.src: W: no-buildroot-tag
ibus-table-translit.noarch: W: spelling-error %description -l en_US ua -> ya, us, u

+ source verified with upstream url as (sha1sum)
a6b347fa2f127911ebe8a1c5752382cb1df1c6b2  ibus-table-others-1.3.0.20100907.tar.gz
a6b347fa2f127911ebe8a1c5752382cb1df1c6b2  ibus-table-others-1.3.0.20100907.tar.gz.srpm

Suggestions:
1) drop the following options to %configure
--disable-static, --prefix

2) Drop the cleaning of buildroot in %install, that mean following line
%__rm -rf $RPM_BUILD_ROOT

3) Good not to use macros for make 

4) Drop the %clean section

5) I will suggest dropping %doc from all subpackages as those already requires main package and adding following
%files
%doc AUTHORS COPYING README

6) Drop the versions from BR: and R: for ibus-table and ibus-table-devel, as we already used to build the required dependencies in release and then this package.

Comment 10 Naveen Kumar 2010-10-19 13:14:01 UTC
Updated Spec URL: http://nkumar.fedorapeople.org/ibus-table-others/1.3.0.20100907-5/ibus-table-others.spec

Updated SRPM URL: http://nkumar.fedorapeople.org/ibus-table-others/1.3.0.20100907-5/ibus-table-others-1.3.0.20100907-5.fc13.src.rpm

Koji Scratch Build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2543046


* Tue Oct 19 2010 Naveen Kumar <nkumar> - 1.3.0.20100907-5
- dropped --disable-static & --prefix from configure
- Dropped the cleaning of buildroot
- macros removed from make
- clean section dropped
- doc moved to main package
- versions from BuildRequires and Requires removed

Comment 11 Parag AN(पराग) 2010-10-20 06:35:51 UTC
There looks some problem with translit database. Please look into fixing that issue first.

APPROVED.

Comment 12 Naveen Kumar 2010-10-20 09:09:38 UTC
Thanks Parag! Yep I will try and fix it as soon as possible.

Comment 13 Naveen Kumar 2010-10-20 09:22:06 UTC
New Package CVS Request
=======================
Package Name: ibus-table-others
Short Description: The package contains various IBus-Tables which include languages of Latin-America, Europe, Southeast Asia, as well as math and other symbols
Owners: nkumar
Branches: F-13 F-14 rawhide
InitialCC: i18n-team

Comment 14 Kevin Fenzi 2010-10-21 13:28:07 UTC
Git done (by process-git-requests).

Comment 15 Naveen Kumar 2010-10-22 09:08:30 UTC

(In reply to comment #14)
> Git done (by process-git-requests).

Thanks for the git :)


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