[Bug 189889] Review Request: vkeybd - Virtual MIDI Keyboard

bugzilla at redhat.com bugzilla at redhat.com
Mon Sep 25 17:40:42 UTC 2006


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: vkeybd - Virtual MIDI Keyboard


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





------- Additional Comments From green at redhat.com  2006-09-25 13:40 EST -------
(In reply to comment #13) 
> - please do not use a full path in the .desktop file in the Exec line because
> this make the path to the binary hardcoded.

Fixed.

> - there are some warnings in build.log in mock: sffile.c:122: warning: ignoring
> return value of 'fread', declared with attribute warn_unused_result - see them
> all in the attachment.

I'll let upstream know.

> - ChangeLog is not packaged

Fixed.

> - did you ask upstream to include your icon / desktop files? There is already an
> desktop file in the upstream tarball, so maybe upstream will include your
> improved desktop file.

I'll do that.
 
> - the lash patch does not patch the README properly(LADCCA is still mentioned):
> 
> -  --ladcca bool
> +  --lash bool
>         Specify the support of LADCCA.  Give yes or no as the

Fixed.

> - the manpage does not mention the --lash option (the upstream version not the
> --ladcca option)

I'll report upstream.

> - have you submitted lash patch to upstream? (Just out of curiosity, what are
> the advantages of lash against ladcca?

LADCCA is dead.  LASH is the new LADCCA.

> - some files have strange permissions, but I don't know whether or not this
> needs to be fixed:
> $ rpm -vql vkeybd | grep -- -r--r--r
> -r--r--r--    1 root    root             2278 Sep 19 23:41
> /usr/share/man/man1/vkeybd.1.gz

Fixed.

> -r--r--r--    1 root    root             5765 Sep 19 23:41
> /usr/share/vkeybd/vkeybd.list
> -r--r--r--    1 root    root              282 Sep 19 23:41
> /usr/share/vkeybd/vkeybdmap
> -r--r--r--    1 root    root              590 Sep 19 23:41
> /usr/share/vkeybd/vkeybdmap-german

I didn't change these.

> - changing %{_datadir}/vkeybd to %{_datadir}/vkeybd/ in %files makes it more
> obvious that an directory is meant

Done.

I also updated the .desktop file as per comment #14.

Updated bits here:

Spec URL: http://people.redhat.com/green/FE/FC5/vkeybd.spec
SRPM URL: http://people.redhat.com/green/FE/FC5/vkeybd-0.1.17-8.src.rpm


-- 
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.




More information about the Fedora-package-review mailing list