Bug 238499 - Review Request: kio_p7zip - Kio-slave for viewing 7zip files using p7zip
Review Request: kio_p7zip - Kio-slave for viewing 7zip files using p7zip
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Tom "spot" Callaway
Fedora Package Reviews List
: Reopened
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-04-30 17:21 EDT by Kelly Miller
Modified: 2007-11-30 17:12 EST (History)
1 user (show)

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


Attachments (Terms of Use)

  None (edit)
Description Kelly Miller 2007-04-30 17:21:51 EDT
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 17:47:32 EDT
Just a couple of really minor issues with this package:

rpmlint says:

W: kio_p7zip wrong-file-end-of-line-encoding
/usr/share/doc/kio_p7zip-0.3.1/ChangeLog
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
recode()
{
        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 18:23:55 EDT
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 19:19:10 EDT
Okay, I updated both the spec and source files.

Spec URL: http://crystalsanctuary.rpgsource.net/packages/specs/kio_p7zip.spec
SRPM URL: 
http://crystalsanctuary.rpgsource.net/packages/source/kio_p7zip-0.3.1-2fc6.src.rpm
Comment 4 Tom "spot" Callaway 2007-04-30 22:26:25 EDT
- 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 07:38:54 EDT
> I assume the .la files are really needed for KDE?

In most cases, yes.
Comment 6 Kelly Miller 2007-05-01 13:07:28 EDT
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
SRPM URL: 
http://crystalsanctuary.rpgsource.net/packages/source/kio_p7zip-0.3.1-3fc6.src.rpm
Comment 7 Kelly Miller 2007-05-16 19:56:23 EDT
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 00:24:16 EDT
cvs done
Comment 9 Kelly Miller 2007-05-28 16:41:11 EDT
Package Change Request
======================
Package Name: kio_p7zip
New Branches: FC-7
Comment 10 Dennis Gilmore 2007-05-28 16:59:33 EDT
cvs done
Comment 11 Kelly Miller 2007-06-01 11:52:13 EDT
Closing until next release.

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