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

[Red Hat Bugzilla]

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

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

Summary: Review Request: vkeybd - Virtual MIDI Keyboard
           Product: Fedora Extras
           Version: devel
          Platform: All
        OS/Version: Linux
            Status: NEW
          Severity: normal
          Priority: normal
         Component: Package Review
        AssignedTo: [e-mail]
        ReportedBy: [e-mail]
         QAContact: [e-mail]

Spec URL: http://people.redhat.com/green/FE/FC5/vkeybd.spec
SRPM URL: http://people.redhat.com/green/FE/FC5/vkeybd-0.1.17-3.src.rpm
Description: 
This is a virtual keyboard for AWE, MIDI and ALSA drivers.
It's a simple fake of a MIDI keyboard on X-windows system.
Enjoy a music with your mouse and "computer" keyboard :-)

Adapted from old ccmra spec file.

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

[Red Hat Bugzilla]

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
OtherBugsDependingO|                            |177841
              nThis|                            |

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

[Red Hat Bugzilla]

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
OtherBugsDependingO|177841                      |
              nThis|                            |

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

[Red Hat Bugzilla]

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 [e-mail]  2006-05-18 22:42 EST -------
Added nando's vkeybd icon.  Minor cleanups.

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

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

[Red Hat Bugzilla]

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 [e-mail]  2006-06-01 09:31 EST -------
Not a Review but some Chnages you need in SPEC :-
1) Use of dist tag in Release tag
2) Consider using make %{?_smp_mflags} in %build
3) Include License file

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

[Red Hat Bugzilla]

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 [e-mail]  2006-06-01 11:08 EST -------
(In reply to comment #2)
> Not a Review but some Chnages you need in SPEC :-

Thanks for the comments. New 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-5.src.rpm

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

[Red Hat Bugzilla]

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 [e-mail]  2006-08-23 12:10 EST -------
Some source files do not have an Copyright / GPL header:

fskip.c
malloc.c
itypes.h
util.h

About the license file:
MUST: If (and only if) the source package includes the text of the license(s) in
its own file, then that file, containing the text of the license(s) for the
package must be included in %doc.

Please ask upstream to add the license and add copyright headers to the
mentioned files.

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

[Red Hat Bugzilla]

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 [e-mail]  2006-08-23 12:15 EST -------
I noticed something else:

A BR is: tcl-devel >= 8.4 and in %build you use TCL_VERSION=8.4
It seems to me that because of this the spec file will not work for tcl-devel
version 8.6 so you may better use tcl-devel = 8.4 imho.

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

[Red Hat Bugzilla]

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 [e-mail]  2006-08-23 12:17 EST -------
Sorry for writing so many comments, I forgot to mention that rpmlint shows an error:

E: vkeybd non-executable-script /usr/share/vkeybd/vkeybd.tcl 0444

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

[Red Hat Bugzilla]

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 [e-mail]  2006-09-17 11:41 EST -------
(In reply to comment #4)
> Please ask upstream to add the license and add copyright headers to the
> mentioned files.

I've asked upstream.

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

[Red Hat Bugzilla]

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 [e-mail]  2006-09-19 08:02 EST -------
(In reply to comment #4)
> Please ask upstream to add the license and add copyright headers to the
> mentioned files.

This was the response:
"I don't mind to add but am too lazy to release a newer version for
such small pieces.  The license issue is written in README."

So, should I just remove the COPYING file I added and proceed?

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

[Red Hat Bugzilla]

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 [e-mail]  2006-09-19 10:16 EST -------
just remove your COPYING file and proceed. Hopefully there will be a new 
release sometime with the file added.

Here are some other items that need to be taken care of:
- move the .desktop file to a separate file
- I think the invocation of "update-desktop-database %{_datadir}/applications" 
is not needed. According to 
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets?highlight=%28updat... 
it is only need when the .desktop file has an MimeType entry
- then you do not need 
"Requires(post):   desktop-file-utils" and
"Requires(postun): desktop-file-utils"
- I am not sure, but $RPM_BUILD_ROOT/%{_datadir}/icons/vkeybd.png seems not 
the be the correct location for the icon since on my system all other icons 
are in subdirectories of icons, e.g. hicolor/<size>
- alsa-lib-devel, jack-audio-connection-kit-devel and e2fsprogs-devel can be 
removed from BuildRequires since they are already required by lash-devel
- tcl-devel can be removed from BuildRequires since it is already required by 
tk-devel if the versioned dependency can be done with tk-devel instead of 
tcl-devel

and please do not forget the issues in comment 5 and 6

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

[Red Hat Bugzilla]

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 [e-mail]  2006-09-19 15:22 EST -------
I've fixed everything mentioned so far and more....

New 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-6.src.rpm

Thanks.

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

[Red Hat Bugzilla]

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 [e-mail]  2006-09-19 18:20 EST -------
The specfile is not up to date.

The requires for gtk should be removed, see:
http://fedoraproject.org/wiki/Packaging/ScriptletSnippets#head-7103f6c38d1b5...

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

[Red Hat Bugzilla]

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 [e-mail]  2006-09-19 18:27 EST -------
(In reply to comment #11)
> The requires for gtk should be removed, see:

D'oh!  Thanks for pointing that out.

New 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-7.src.rpm

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

[Red Hat Bugzilla]

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
         AssignedTo|[e-mail] |[e-mail]
OtherBugsDependingO|163776                      |163778
              nThis|                            |

------- Additional Comments From [e-mail]  2006-09-19 19:47 EST -------
It is too late for me to do a complete review now but there are still some minor
issues:

- please do not use a full path in the .desktop file in the Exec line because
this make the path to the binary hardcoded.

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

- ChangeLog is not packaged

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

- 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

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

- have you submitted lash patch to upstream? (Just out of curiosity, what are
the advantages of lash against 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
-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

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

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

[Red Hat Bugzilla]

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 [e-mail]  2006-09-19 19:56 EST -------
Just noticed something else:

You can remove these lines from your spec
  --add-category Application                    
  --add-category AudioVideo 
and add to the .desktop file:

Categories=Application;AudioVideo;
or
Categories=Application;AudioVideo;Audio;Midi;Music;
This makes the .desktop file even more useful for inclusion in upstream.

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

[Red Hat Bugzilla]

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 [e-mail]  2006-09-19 19:57 EST -------
Created an attachment (id=136691)
 --> (https://bugzilla.redhat.com/bugzilla/attachment.cgi?id=136691&action=vie...)
mock build.log from revision 6 for development

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

[Red Hat Bugzilla]

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 [e-mail]  2006-09-20 04:42 EST -------
The 0444 permission on some files do not need to be changed.

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

[Red Hat Bugzilla]

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 [e-mail]  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

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

[Red Hat Bugzilla]

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
OtherBugsDependingO|163778                      |163779
              nThis|                            |

------- Additional Comments From [e-mail]  2006-10-17 06:48 EST -------
- I just noticed that it is not the latest version, 0.1.17a is available at
http://www.alsa-project.org/~iwai/vkeybd-0.1.17a.tar.bz2 with french keyboard
layout in vkeybdmap-french
- Requires: jack-audio-connection-kit is not needed because lash already depends
on it.

Builds fine in mock and rpmlint does not complain, naming, licenses, files and
scriptlets are ok.
md5sum: 3be348329a5aac3cdc21f89458d9369a matches.

So remove Requires: jack-audio-connection-kit and update to the latest version
and the package is approved.

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

[Red Hat Bugzilla]

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

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|                            |NEXTRELEASE

------- Additional Comments From [e-mail]  2006-10-21 13:14 EST -------
(In reply to comment #18)
> So remove Requires: jack-audio-connection-kit and update to the latest version
> and the package is approved.

Done and done.  Thanks!

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

[Red Hat Bugzilla]

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/show_bug.cgi?id=189889

[e-mail] changed:

What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |medium
           Priority|normal                      |medium
            Product|Fedora Extras               |Fedora
            Version|devel                       |rawhide