Bug 585902 - Review Request: cardpeek - Tool to read the contents of smart cards
Review Request: cardpeek - Tool to read the contents of smart cards
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Parag AN(पराग)
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2010-04-26 07:29 EDT by Kalev Lember
Modified: 2010-05-06 13:00 EDT (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2010-05-06 13:00:29 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
panemade: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Kalev Lember 2010-04-26 07:29:36 EDT
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 04:42:07 EDT
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 06:06:45 EDT
Thank you for reviewing this, Parag.

* Mon May 03 2010 Kalev Lember <kalev@smartlink.ee> - 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 03:05:17 EDT
APPROVED.
Comment 4 Kalev Lember 2010-05-04 03:34:16 EDT
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 11:47:47 EDT
CVS done (by process-cvs-requests.py).
Comment 6 Kalev Lember 2010-05-06 13:00:29 EDT
Package imported and built.

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