[Bug 188369] Review Request: ctapi-cyberjack

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: ctapi-cyberjack


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=188369





------- Additional Comments From j.w.r.degoede at hhs.nl  2006-05-09 15:59 EST -------
Sorry Frank,

But there are still some issues. My fault completly:
-I forgot to mention in my review that you should also add an -soname option
 to the ifd handler "ld" command in the Makefile. This is a should fix though
 and doesn't block approval

-The %post and %postun scripts for pcsc however do need some minor changes and
 that I do concider a blocker:
 -The comments should be below the %post resp  %postun not above. As it is now
  the comment above %post will become part of %install (and do nothing there)
  and the comment above %postun will become part of %post (try rpm -q --scripts)
  instead of %postun.
 -A bigger problem is that each command in a %post script should end with "|| :"
  Because rpm will concider a package as not installed (or not removed!) if the
  script fails and bash uses the exit of the last command as script exit.
  So change: "/sbin/service pcscd condrestart" to
  "/sbin/service pcscd condrestart || :"
 -You now have the Requires(post[un]): /usr/sbin/update-reader.conf as I 
  adviced, but you no longer use those. I guess that your initial solution of
  just requiring pcsc-lite was better. Sorry about this, I though the explicit
  /usr/sbin/update-reader.conf Requires would be better thinking that maybe one
  day we will have more then one pcsc implementation, but that seems highly 
  unlikely. So just move back to "Requires: pcsc-lite" for the -pcsc subpackage
  (_Sorry_).

With these few easy fixes we really should be there! I also notices some nasty
64 bit related warnings, but I've checked the relevant part of the sources and
they seem harmless. Looking forward to -12 and to approving it!


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.