[Bug 781688] Review Request: lv2-mdaEPiano - LV2 port of the MDA VST piano plugin

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


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

Orcan Ogetbil <oget.fedora at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |MODIFIED
         AssignedTo|nobody at fedoraproject.org    |oget.fedora at gmail.com
               Flag|                            |fedora-review?

--- Comment #3 from Orcan Ogetbil <oget.fedora at gmail.com> 2012-01-23 21:19:38 EST ---
Thanks, here is the full review. There are a few issues we need to go over.:

* I could not match the sources with the tarball you provided. Also your
comments checking out instructions have a different git hash.
   $ sha1sum ~/rpmbuild/SOURCES/lv2-mdaEPiano-0.0.0-git5a4ab089.tar.bz2 
   51c92dcfaa0b2342bd7281e430aa05d7187a0c29 
/home/orcan/rpmbuild/SOURCES/lv2-mdaEPiano-0.0.0-git5a4ab089.tar.bz2
   $ sh ~/rpmbuild/SOURCES/lv2-mdaEPiano-snapshot.sh 5a4ab089
   ...
   Fetched git revision: 5a4ab089
   Written: lv2-mdaEPiano-0.0.0-git5a4ab089.tar.bz2
   $ sha1sum lv2-mdaEPiano-0.0.0-git5a4ab089.tar.bz2
   dff89657f88ce8d514a1e38433aaada71f51fa2a 
lv2-mdaEPiano-0.0.0-git5a4ab089.tar.bz2

! You can probably remove the .git stuff from the source tree in your snapshot
script.

- rpmlint says: lv2-mdaEPiano.src: W: invalid-url Source0:
lv2-mdaEPiano-0.0.0-git5a4ab089.tar.bz2
   This can be ignored.

* The following are redundant
   %post -p /usr/bin/ldconfig
   %postun -p /usr/bin/ldconfig
since we are not insntalling anything in the ld search path.

? I found  binary file ./src/data/samples.raw in the tarball. Do you know what
this is? 

? Is there a justification for the version 0.0.0? If not, we shoud probably use
plain 0. See the kismet example a
  
http://fedoraproject.org/wiki/Packaging:NamingGuidelines#Pre-Release_packages

* The README.md file says the license is GPLv3+. Please fix this.

* Fedora specific build flags are not honored. In particular, there is a -O
that overrides our -O2.

* The package should BR lv2-c++-tools-static instead of lv2-c++-tools-devel
since it uses the static library.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.