Bug 670090 - Review Request: bicon - Bidirectional Console
Summary: Review Request: bicon - Bidirectional Console
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Daiki Ueno
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2011-01-17 07:19 UTC by fujiwara
Modified: 2011-01-25 20:56 UTC (History)
5 users (show)

Fixed In Version: bicon-0.2.0-1.fc13
Clone Of:
Environment:
Last Closed: 2011-01-25 20:55:10 UTC
Type: ---
Embargoed:
dueno: fedora-review+
j: fedora-cvs+


Attachments (Terms of Use)

Description fujiwara 2011-01-17 07:19:28 UTC
Spec URL: http://fujiwara.fedorapeople.org/bicon/bicon.spec
SRPM URL: http://fujiwara.fedorapeople.org/bicon/bicon-0.2.0-1.src.rpm
Description:
BiCon is the bidirectional console as presented by Arabeyes.

Comment 1 fujiwara 2011-01-17 07:28:59 UTC
Forgot dist tag in NVR:
SRPM URL: http://fujiwara.fedorapeople.org/bicon/bicon-0.2.0-1.fc15.src.rpm

Comment 2 fujiwara 2011-01-17 07:35:36 UTC
Scratch build is done:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2725386

Comment 3 Daiki Ueno 2011-01-17 08:59:28 UTC
I'm trying to review.

- rpmlint reports several errors and warnings with binary RPMs

 bicon.x86_64: E: standard-dir-owned-by-package /usr/share/man
 bicon.x86_64: E: standard-dir-owned-by-package /usr/share/man/man1

Please exclude those directories from %files.

 bicon.x86_64: E: non-readable /usr/share/doc/bicon-0.2.0/README 0600L
 bicon.x86_64: E: non-readable /usr/share/doc/bicon-0.2.0/COPYING 0600L
 bicon.x86_64: E: non-readable /usr/share/doc/bicon-0.2.0/AUTHORS 0600L

Please give proper permissions to %docs.

 bicon.x86_64: W: devel-file-in-non-devel-package /usr/lib64/bicon/libbjoining.so
 bicon.x86_64: W: devel-file-in-non-devel-package /usr/lib64/bicon/libbconsole.so

Please exclude those symlinks from the base package.

- it would be nice to preserve timestamps when install.
https://fedoraproject.org/wiki/Packaging:Guidelines#Timestamps

Consider using something like:

 make DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p" install

- you can drop BuildRoot stuff
https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag

- consider having a separate package for fonts
https://fedoraproject.org/wiki/Packaging:Guidelines#Avoid_bundling_of_fonts_in_other_packages

I think this might not apply since the package contains only 4 fonts and each of them is about 4KB, but if you plan to subpackage them, you could check terminus-fonts-console.

Comment 4 fujiwara 2011-01-18 02:39:31 UTC
(In reply to comment #3)
> I'm trying to review.

Thanks. I updated spec file:
Spec URL: http://fujiwara.fedorapeople.org/bicon/bicon.spec
SRPM URL: http://fujiwara.fedorapeople.org/bicon/bicon-0.2.0-2.fc15.src.rpm

Scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2727535

 
>  bicon.x86_64: E: non-readable /usr/share/doc/bicon-0.2.0/README 0600L
>  bicon.x86_64: E: non-readable /usr/share/doc/bicon-0.2.0/COPYING 0600L
>  bicon.x86_64: E: non-readable /usr/share/doc/bicon-0.2.0/AUTHORS 0600L

I didn't get the same error with my scratch build but I added chmod at the moment.

> - consider having a separate package for fonts
> https://fedoraproject.org/wiki/Packaging:Guidelines#Avoid_bundling_of_fonts_in_other_packages
> 
> I think this might not apply since the package contains only 4 fonts and each
> of them is about 4KB, but if you plan to subpackage them, you could check
> terminus-fonts-console.

OK, I think probably sub-packages are good because fonts and keymaps are not used in TERM=xterm.

Comment 5 Daiki Ueno 2011-01-18 07:03:48 UTC
(In reply to comment #4)

> >  bicon.x86_64: E: non-readable /usr/share/doc/bicon-0.2.0/README 0600L
> >  bicon.x86_64: E: non-readable /usr/share/doc/bicon-0.2.0/COPYING 0600L
> >  bicon.x86_64: E: non-readable /usr/share/doc/bicon-0.2.0/AUTHORS 0600L
> 
> I didn't get the same error with my scratch build but I added chmod at the
> moment.

Sorry, my bad.  That was because I used "fakeroot rpmbuild --rebuild" to check the SRPM on F-14.  Please feel free to revert the change when you import the package into the git.

BTW, according to COPYING, bicon/pty_spawn.c is covered by the Python license. 
The License tag should be fixed:
https://fedoraproject.org/wiki/Packaging:LicensingGuidelines#Multiple_Licensing_Scenarios

APPROVED.

Comment 6 fujiwara 2011-01-18 10:27:44 UTC
New Package SCM Request
=======================
Package Name: bicon
Short Description: Bidirectional Console
Owners: fujiwara
Branches: F-13, F-14
InitialCC: i18n-team


Scratch build:
http://koji.fedoraproject.org/koji/taskinfo?taskID=2728197

Comment 7 Jason Tibbitts 2011-01-18 14:03:13 UTC
Git done (by process-git-requests).

Comment 9 Fedora Update System 2011-01-20 03:49:53 UTC
bicon-0.2.0-1.fc14 has been submitted as an update for Fedora 14.
https://admin.fedoraproject.org/updates/bicon-0.2.0-1.fc14

Comment 10 Fedora Update System 2011-01-20 03:50:00 UTC
bicon-0.2.0-1.fc13 has been submitted as an update for Fedora 13.
https://admin.fedoraproject.org/updates/bicon-0.2.0-1.fc13

Comment 11 Fedora Update System 2011-01-20 19:54:03 UTC
bicon-0.2.0-1.fc14 has been pushed to the Fedora 14 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update bicon'.  You can provide feedback for this update here: https://admin.fedoraproject.org/updates/bicon-0.2.0-1.fc14

Comment 12 Fedora Update System 2011-01-25 20:55:05 UTC
bicon-0.2.0-1.fc14 has been pushed to the Fedora 14 stable repository.  If problems still persist, please make note of it in this bug report.

Comment 13 Fedora Update System 2011-01-25 20:56:42 UTC
bicon-0.2.0-1.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.