Bug 585902

Summary:	Review Request: cardpeek - Tool to read the contents of smart cards
Product:	[Fedora] Fedora	Reporter:	Kalev Lember <kalevlember>
Component:	Package Review	Assignee:	Parag AN(पराग) <panemade>
Status:	CLOSED NEXTRELEASE	QA Contact:	Fedora Extras Quality Assurance <extras-qa>
Severity:	medium	Docs Contact:
Priority:	medium
Version:	rawhide	CC:	fedora-package-review, notting, panemade
Target Milestone:	---	Flags:	panemade: fedora-review+ kevin: fedora-cvs+
Target Release:	---
Hardware:	All
OS:	Linux
Whiteboard:
Fixed In Version:		Doc Type:	Bug Fix
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2010-05-06 17:00:29 UTC	Type:	---
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

Description Kalev Lember 2010-04-26 11:29:36 UTC

Spec URL: http://kalev.fedorapeople.org/cardpeek.spec
SRPM URL: http://kalev.fedorapeople.org/cardpeek-0.5-1.fc14.src.rpm
Description:
Cardpeek is a tool to read the contents of ISO7816 smart cards. It
features a GTK GUI to represent card data is a tree view, and is
extendable with a scripting language (LUA).

Comment 1 Parag AN(पराग) 2010-05-03 08:42:07 UTC

Review:
+ package builds in mock (rawhide i686).
koji Build =>http://koji.fedoraproject.org/koji/taskinfo?taskID=2156995
+ rpmlint output for SRPM and for RPM.
cardpeek.src: W: spelling-error %description -l en_US extendable -> expendable, extend able, extend-able
cardpeek.src: W: invalid-url Source0: http://cardpeek.googlecode.com/files/cardpeek-0.5.tar.gz HTTP Error 404: Not Found
cardpeek.x86_64: W: spelling-error %description -l en_US extendable -> expendable, extend able, extend-able
==> Ignore this.

+ source files match upstream url (sha1sum)
0d5cb13821b21ca6367d26b16f447f2ad6b75b82  cardpeek-0.5.tar.gz
+ package meets naming and packaging guidelines.
+ specfile is properly named, is cleanly written
+ Spec file is written in American English.
+ Spec file is legible.
+ dist tag is present.
+ license is open source-compatible.
+ License text is included in package.
+ %doc is present.
+ BuildRequires are proper.
+ %clean is present.
+ package installed properly.
+ Macro use appears rather consistent.
+ no headers or static libraries.
+ no .pc file present.
+ no -devel subpackage
+ no .la files.
+ no translations are available
+ Does owns the directories it creates.
+ gtk-update-icon-cache scriptlets present.
+ no duplicates in %files.
+ file permissions are appropriate.
+ Desktop file is present and installed correctly.
+ A GUI application.

Suggestions:-
1) As per http://lists.fedoraproject.org/pipermail/devel/2010-March/133523.html, you need to add in desktop file
StartupNotify=true

2)it will be good if patch cardpeek-executable-stack.patch would have committed in upstream first. Have you contacted upstream personally to include this patch besides submitting it as an issue?

3) To keep timestamps of files getting installed from tarball, you should use 
make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p"


4)also, you can add to %description
The tool currently reads the contents of :
    * EMV cards
    * Calypso public transport cards (such as Navigo)
    * Moneo ePurse cards
    * Vitale 2 French health cards.

Comment 2 Kalev Lember 2010-05-03 10:06:45 UTC

Thank you for reviewing this, Parag.

* Mon May 03 2010 Kalev Lember <kalev> - 0.5-2
- Set StartupNotify=true in desktop file
- Use INSTALL="install -p" to keep timestamps
- Updated description

Spec URL: http://kalev.fedorapeople.org/cardpeek.spec
SRPM URL: http://kalev.fedorapeople.org/cardpeek-0.5-2.fc14.src.rpm
Scratch build: http://koji.fedoraproject.org/koji/taskinfo?taskID=2157432

(In reply to comment #1)
> 2)it will be good if patch cardpeek-executable-stack.patch would have committed
> in upstream first. Have you contacted upstream personally to include this patch
> besides submitting it as an issue?

Yes, I sent an e-mail the same day I submitted those patches and got the following response in private mail a few days later:
> Hi,
> 
> Thank you for your message and your good quality feedback. I really appreciate
> it.
> 
> I will include your patches in the next realease of cardpeek.
> 
> L1L1

No changes have been commited in upstream repository so far.

Comment 3 Parag AN(पराग) 2010-05-04 07:05:17 UTC

APPROVED.

Comment 4 Kalev Lember 2010-05-04 07:34:16 UTC

New Package CVS Request
=======================
Package Name: cardpeek
Short Description: Tool to read the contents of smart cards
Owners: kalev
Branches: F-12 F-13
InitialCC:

Comment 5 Kevin Fenzi 2010-05-06 15:47:47 UTC

CVS done (by process-cvs-requests.py).

Comment 6 Kalev Lember 2010-05-06 17:00:29 UTC

Package imported and built.