Bug 238499 - Review Request: kio_p7zip - Kio-slave for viewing 7zip files using p7zip
Summary: Review Request: kio_p7zip - Kio-slave for viewing 7zip files using p7zip
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
(Show other bugs)
Version: rawhide
Hardware: All Linux
Target Milestone: ---
Assignee: Tom "spot" Callaway
QA Contact: Fedora Package Reviews List
Keywords: Reopened
Depends On:
TreeView+ depends on / blocked
Reported: 2007-04-30 21:21 UTC by Kelly Miller
Modified: 2007-11-30 22:12 UTC (History)
1 user (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Last Closed: 2007-06-01 15:52:13 UTC
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tcallawa: fedora-review+
dennis: fedora-cvs+

Attachments (Terms of Use)

Description Kelly Miller 2007-04-30 21:21:51 UTC
Spec URL: http://crystalsanctuary.rpgsource.net/packages/specs/kio_p7zip.spec
SRPM URL: http://crystalsanctuary.rpgsource.net/packages/source/kio_p7zip-0.3.1-1fc6.src.rpm
Description: This is a kioslave for KDE to handle 7zip files.

Comment 1 Tom "spot" Callaway 2007-04-30 21:47:32 UTC
Just a couple of really minor issues with this package:

rpmlint says:

W: kio_p7zip wrong-file-end-of-line-encoding
W: kio_p7zip wrong-file-end-of-line-encoding /usr/share/doc/kio_p7zip-0.3.1/README
E: kio_p7zip zero-length /usr/share/doc/kio_p7zip-0.3.1/NEWS

Basically, you don't need to package the empty NEWS file.
Also, you'll want to use iconv to fix those file encodings:

(add this to %install)

# Fix file encoding
        iconv -f "$2" -t utf-8 < "$1" > "${1}_"
        mv -f "${1}_" "$1"
recode ChangeLog iso-8859-1
recode README iso-8859-1

Last but not least, I assume the .la files are really needed for KDE? I'm not a
KDE user, but if they can be avoided, that would be ideal. Adding Rex to the CC
for this, as he'll know.

Fix those minor items, show me a new spec/SRPM, and I'll do the rest of the review.

Comment 2 Christopher Stone 2007-04-30 22:23:55 UTC
Actually, the files are encoded properly, its just the line endings which are
incorrect.  You can fix it with just:

# Fix end-of-line-encoding
sed -i 's/\r//' ChangeLog
sed -i 's/\r//' README

Comment 3 Kelly Miller 2007-04-30 23:19:10 UTC
Okay, I updated both the spec and source files.

Spec URL: http://crystalsanctuary.rpgsource.net/packages/specs/kio_p7zip.spec

Comment 4 Tom "spot" Callaway 2007-05-01 02:26:25 UTC
- rpmlint checks return nothing
- package meets naming guidelines
- package meets packaging guidelines
- license (GPL) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86_64)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime

APPROVED. If your other sponsorship doesn't happen for any reason, lemme know.

Comment 5 Rex Dieter 2007-05-01 11:38:54 UTC
> I assume the .la files are really needed for KDE?

In most cases, yes.

Comment 6 Kelly Miller 2007-05-01 17:07:28 UTC
I didn't notice any problems, but I've taken the line removing the .la files 
out of the spec just in case.

Spec URL: http://crystalsanctuary.rpgsource.net/packages/specs/kio_p7zip.spec

Comment 7 Kelly Miller 2007-05-16 23:56:23 UTC
New Package CVS Request
Package Name: kio_p7zip
Short Description: Kio-slave for reading 7zip files
Owners: lightsolphoenix@gmail.com
Branches: FC-6
InitialCC: lightsolphoenix@gmail.com

Comment 8 Dennis Gilmore 2007-05-17 04:24:16 UTC
cvs done

Comment 9 Kelly Miller 2007-05-28 20:41:11 UTC
Package Change Request
Package Name: kio_p7zip
New Branches: FC-7

Comment 10 Dennis Gilmore 2007-05-28 20:59:33 UTC
cvs done

Comment 11 Kelly Miller 2007-06-01 15:52:13 UTC
Closing until next release.

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