Bug 556988 - Review Request: ibus-fbterm - IBus front-end for fbterm
Summary: Review Request: ibus-fbterm - IBus front-end for fbterm
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
low
medium
Target Milestone: ---
Assignee: Ding-Yi Chen
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2010-01-20 01:13 UTC by caius.chance
Modified: 2010-03-05 03:38 UTC (History)
6 users (show)

Fixed In Version: ibus-fbterm-0.9.1-7.fc13
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2010-03-05 03:38:55 UTC
dchen: fedora-review+
tibbs: fedora-cvs+


Attachments (Terms of Use)

Description caius.chance 2010-01-20 01:13:11 UTC
Spec URL: http://kaio.fedorapeople.org/packaging/ibus-fbterm-0.9.1.tar.gz
SRPM URL: http://kaio.fedorapeople.org/packaging/ibus-fbterm.spec
Description: IBus front-end for fbterm.

Comment 1 Ding-Yi Chen 2010-01-20 02:04:55 UTC
Some quick comments:
1. I find upstream's description is more appropriate. :-P

2. rpmlint reports 
./RPMS/x86_64/ibus-fbterm-0.9.1-1.fc12.x86_64.rpm
ibus-fbterm.x86_64: E: zero-length /usr/share/doc/ibus-fbterm-0.9.1/AUTHORS
   but it is up to you whether to fix this error or not.

3. you may need to patch configure.ac, namely
PKG_CHECK_MODULES([ibus], [ibus-1.0 >= 1.2.0])
   As this lead to koji build error.
   Maybe it's best to ask upstream to fix it.

Comment 2 Ding-Yi Chen 2010-01-20 02:27:29 UTC
Regarding to #1, point 3.
Guess that's because ibus-devel is not in BuildRequired

Comment 3 caius.chance 2010-01-20 02:38:48 UTC
Spec URL: http://kaio.fedorapeople.org/pkgs/ibus-fbterm-0.9.1-2.fc12.src.rpm
SRPM URL: http://kaio.fedorapeople.org/packaging/ibus-fbterm.spec
Description: IBus front-end for fbterm.

Comment 4 Ding-Yi Chen 2010-01-22 05:17:17 UTC
Two more questions:

1. Any reason you limit the BuildArch to i586 and x86_64?

2. ibus-fbterm-lanuch works with bash, but not tcsh,
   following patch can fix that.

--- /usr/bin/ibus-fbterm-launch.orig	2010-01-22 12:39:58.822703153 +1000
+++ /usr/bin/ibus-fbterm-launch	2010-01-22 14:25:26.286694006 +1000
@@ -12,7 +12,7 @@
 	exit 1
 fi
 
-eval `dbus-launch --auto-syntax`
+eval `dbus-launch --sh-syntax`
 
 export DISPLAY=:9${TTY_NUM}.0

Comment 5 caius.chance 2010-01-22 06:14:41 UTC
(In reply to comment #4)
> 1. Any reason you limit the BuildArch to i586 and x86_64?

Just because "noarch" as value doesn't work.

> 2. ibus-fbterm-lanuch works with bash, but not tcsh,
>    following patch can fix that.
> 
> --- /usr/bin/ibus-fbterm-launch.orig 2010-01-22 12:39:58.822703153 +1000
> +++ /usr/bin/ibus-fbterm-launch 2010-01-22 14:25:26.286694006 +1000
> @@ -12,7 +12,7 @@
>   exit 1
>  fi
> 
> -eval `dbus-launch --auto-syntax`
> +eval `dbus-launch --sh-syntax`
> 
>  export DISPLAY=:9${TTY_NUM}.0    

Will patch and resubmit.

Comment 6 caius.chance 2010-01-22 06:37:13 UTC
Spec URL: http://kaio.fedorapeople.org/pkgs/ibus-fbterm-0.9.1-3.fc12.src.rpm
SRPM URL: http://kaio.fedorapeople.org/packaging/ibus-fbterm.spec
Description: IBus front-end for fbterm.

Comment 7 Ding-Yi Chen 2010-01-25 01:40:53 UTC
Thanks for the pa(In reply to comment #5)
> (In reply to comment #4)
> > 1. Any reason you limit the BuildArch to i586 and x86_64?
> 
> Just because "noarch" as value doesn't work.

It's a C program, no doubt it won't work. :-)
I've built it with koji under f-12 and f-13 without error.
Maybe you can remove the BuildArch line.

Comment 8 caius.chance 2010-02-15 07:32:14 UTC
Spec URL: http://kaio.fedorapeople.org/pkgs/ibus-fbterm-0.9.1-4.fc12.src.rpm
SRPM URL: http://kaio.fedorapeople.org/packaging/ibus-fbterm.spec
Description: IBus front-end for fbterm.

Comment 9 caius.chance 2010-02-16 00:14:29 UTC
Spec URL: http://kaio.fedorapeople.org/pkgs/ibus-fbterm-0.9.1-5.fc12.src.rpm
SRPM URL: http://kaio.fedorapeople.org/packaging/ibus-fbterm.spec
Description: IBus front-end for fbterm.

Comment 10 Jens Petersen 2010-02-17 01:16:26 UTC
I am not sure about the %post and %postun scripts.

Just noticed this new policy:
https://fedoraproject.org/wiki/Privilege_escalation_policy

This probably also applies to fbterm itself (and other consoles).

Comment 11 Ding-Yi Chen 2010-02-23 23:13:30 UTC
1.  Please remove the chmod /dev/fd0 line, as Jens suggests.

It is noble to make it usable out-of-the-box, however, it irritates the nerve of security sense.


2.  Have you noticed that you swapped spec and srpm?  :-)

Comment 12 Caius Chance 2010-02-26 04:09:18 UTC
Spec URL: http://kaio.fedorapeople.org/packaging/ibus-fbterm.spec
SRPM URL: http://kaio.fedorapeople.org/pkgs/ibus-fbterm-0.9.1-6.fc12.src.rpm
Description: IBus front-end for fbterm.

Comment 13 Ding-Yi Chen 2010-02-26 08:07:33 UTC
MUST:
-  rpmlint output is acceptable.

W: incoherent-version-in-changelog 0.9.1-6.fc14 ['0.9.1-6.fc12', '0.9.1-6']
Remove the .fc14 part

+  Package meets naming and packaging guidelines.
+  Package meets licensing guidelines, and match the source license.
+  Source files match upstream.
+  specfile is properly named, is cleanly written
+  Spec file is written in American English.
+  Spec file is legible.
+  dist tag is present.
+  BuildRoot is proper.
+  BuildRequires are proper.
+  Requires are proper.
+  %install starts with rm -rf %{buildroot} (or $RPM_BUILD_ROOT)
+  %clean contains rm -rf %{buildroot} (or $RPM_BUILD_ROOT)
+  %doc files present.
+  %doc files do not interfere runtime application.
+  Macros are consistently used.
+  Package builds in koji.
+  Package contains code or permissible content.
+  Package installed properly.
+  No system library is bundled.
+  Not relocatable, unless proper justification is presented.
+  %files section must include a %defattr(...) line, and file permissions are correct.
+  No duplication in %files
+  File names are in valid UTF-8.
+  Own all directory it creates.
+  Files or directories are not owned by other packages.
+  Large documentation files goes in a -doc subpackage.
+  No .la libtool archives exists.

SHOULD:
+  License text are in separate files.
+  Translations for supported non-English languages if available.
+  Package build in mock.
+  Package can build in all supported architectures.
+  Package runs properly.
+  Scriptlets are sane.
+  Subpackages Requires: %{name} = %{version}-%{release}
+  .pc is in devel subpackage.
+  No direct files dependencies, unless they are in either /etc, /bin, /sbin, /usr/bin, or /usr/sbin

Comment 14 Ding-Yi Chen 2010-02-26 08:09:14 UTC
Summary:
1. W: incoherent-version-in-changelog 0.9.1-6.fc14 ['0.9.1-6.fc12', '0.9.1-6']

Remove the .fc14 part

2. Consider change the Group to: Application/System, as it seems more appropriate.


Otherwise looks like a pass to me.

Regards,

Comment 15 Caius Chance 2010-02-27 19:06:20 UTC
Spec URL: http://kaio.fedorapeople.org/packaging/ibus-fbterm.spec
SRPM URL: http://kaio.fedorapeople.org/pkgs/ibus-fbterm-0.9.1-7.fc12.src.rpm
Description: IBus front-end for fbterm.

Comment 16 Ding-Yi Chen 2010-02-28 23:39:37 UTC
MUST:
+  rpmlint output is acceptable.

APPROVED

Comment 17 Caius Chance 2010-03-01 00:49:09 UTC
New Package CVS Request
=======================
Package Name: ibus-fbterm
Short Description: IBus front-end for fbterm.
Owners: kaio
Branches: F12
InitialCC: i18n@lists.fedoraproject.org

Comment 18 Caius Chance 2010-03-01 06:19:18 UTC
New Package CVS Request
=======================
Package Name: ibus-fbterm
Short Description: IBus front-end for fbterm.
Owners: kaio
Branches: F-12 F-13
InitialCC: i18n-team

Comment 19 Jason Tibbitts 2010-03-01 16:56:40 UTC
CVS done (by process-cvs-requests.py).

Comment 20 Fedora Update System 2010-03-04 07:13:14 UTC
ibus-fbterm-0.9.1-7.fc13 has been submitted as an update for Fedora 13.
http://admin.fedoraproject.org/updates/F13/FEDORA-2010-3453

Comment 21 Fedora Update System 2010-03-05 03:38:49 UTC
ibus-fbterm-0.9.1-7.fc13 has been pushed to the Fedora 13 stable repository.  If problems still persist, please make note of it in this bug report.


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