Bug 1006187 - Review Request: lv2-fabla - an LV2 sampler plugin
Review Request: lv2-fabla - an LV2 sampler plugin
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
Unspecified Unspecified
unspecified Severity unspecified
: ---
: ---
Assigned To: Zbigniew Jędrzejewski-Szmek
Fedora Extras Quality Assurance
:
Depends On:
Blocks: FedoraAudio
  Show dependency treegraph
 
Reported: 2013-09-10 03:47 EDT by Brendan Jones
Modified: 2014-01-24 02:43 EST (History)
2 users (show)

See Also:
Fixed In Version: lv2-fabla-1.1-1.3.20131003git5f2cb26.fc19
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2013-11-10 02:06:21 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
zbyszek: fedora‑review+
limburgher: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Brendan Jones 2013-09-10 03:47:38 EDT
lv2-fabla is a MIDI controllable drum sampler plugin with an ADSR envelope.

LV2 is the latest linux audio plugin standard for use in hosts such as Ardour and Qtractor.

SRPM: http://bsjones.fedorapeople.org/reviews/lv2-fabla-0.0.1-0.2.gite8fb937.fc19.src.rpm
SPEC: http://bsjones.fedorapeople.org/reviews/lv2-fabla.spec

rpmlint ~/rpmbuild/RPMS/x86_64/lv2-fabla-*-0.2* ~/rpmbuild/SPECS/lv2-fabla.spec ~/rpmbuild/SRPMS/lv2-fabla-0.0.1-0.2.gite8fb937.fc19.src.rpm 
/home/bsjones/rpmbuild/SPECS/lv2-fabla.spec: W: invalid-url Source2: lv2-fabla-presets-0.0.1-20130719git4cc9bc4.tar.gz
/home/bsjones/rpmbuild/SPECS/lv2-fabla.spec: W: invalid-url Source0: lv2-fabla-0.0.1-20130908gite8fb937.tar.gz
lv2-fabla.src: W: invalid-url Source2: lv2-fabla-presets-0.0.1-20130719git4cc9bc4.tar.gz
lv2-fabla.src: W: invalid-url Source0: lv2-fabla-0.0.1-20130908gite8fb937.tar.gz
3 packages and 1 specfiles checked; 0 errors, 4 warnings.
Comment 2 Zbigniew Jędrzejewski-Szmek 2013-10-24 20:10:16 EDT
1. The presets part should be split into a noarch package. It has separate sources anyway. The "main" package can then depend on it.

2. I think you can use straightforward urls in Source0 and others:
https://github.com/harryhaaren/openAV-Fabla/archive/%{shortcommit}.tar.gz

The scripts will not be necessary too.

3. Drop %commit, and rename %shortcommit to %commit. Less typing ;)

4. You can remove '-m 0755' from install options: it should be the default.
It might be even simpler to use 'mkdir -p'.
Comment 3 Brendan Jones 2013-10-24 22:18:59 EDT
OK, the long commit is still needed to get the directory.

I have implemented your other recommendations here. Thanks

SRPM: http://bsjones.fedorapeople.org/reviews/lv2-fabla-1.1-1.1.20131003git5f2cb26.fc20.src.rpm
SPEC: http://bsjones.fedorapeople.org/reviews/lv2-fabla.spec
Comment 4 Zbigniew Jędrzejewski-Szmek 2013-10-24 23:37:53 EDT
> OK, the long commit is still needed to get the directory.
True, I didn't see that.

So, why not use two *source* packages for the presets and the sequencer? They have separate repositories upstream, which suggests that they are separate entities.
Comment 5 Brendan Jones 2013-10-24 23:59:59 EDT
Because it is a content package which is useless by itself. I'm not going to submit a review for a bunch of wav files with some metadata. If you have issue with it I'll just drop it entirely, although it would be a shame.
Comment 6 Brendan Jones 2013-10-25 00:01:27 EDT
PS. please redownload the SRPM, may have been uploading again while you were looking
Comment 7 Brendan Jones 2013-10-25 00:12:06 EDT
Removed one of the preset directories (invalid symlinks):

SRPM: http://bsjones.fedorapeople.org/reviews/lv2-fabla-1.1-1.2.20131003git5f2cb26.fc20.src.rpm
SPEC: http://bsjones.fedorapeople.org/reviews/lv2-fabla.spec
Comment 8 Zbigniew Jędrzejewski-Szmek 2013-10-25 19:16:56 EDT
The presets:

Guidelines say: "If the content enhances the OS user experience, then the content is OK to be packaged in Fedora." and "Fedora packages should make every effort to avoid having multiple, separate, upstream projects bundled together in a single package."

This means that the presets can be packaged, but should be separate. I don't see a strong reason to avoid having a separate package here. I agree that it would be a shame not to package the presets. I promise I'll review this trivial package in no time :)

There's also the issue of licensing: afaics, https://github.com/harryhaaren/openAV-presets/ doesn't say anything about the license.
Comment 9 Brendan Jones 2013-10-25 22:48:47 EDT
OK. Removed. Link to the presets repo in the description should suffice.


SRPM: http://bsjones.fedorapeople.org/reviews/lv2-fabla-1.1-1.3.20131003git5f2cb26.fc20.src.rpm
SPEC: http://bsjones.fedorapeople.org/reviews/lv2-fabla.spec

ta
Comment 10 Zbigniew Jędrzejewski-Szmek 2013-10-26 09:49:49 EDT
I files a request to have a license for the other set of presets: https://github.com/harryhaaren/openAV-presets/issues/1, maybe they can be packaged later.

Package looks OK. But please split out the wav files and other non-arch data in a noarch subpackage.
Comment 11 Brendan Jones 2013-10-26 11:51:12 EDT
Thanks

The nature of an lv2 bundle is arch specific by definition (seeing as they live in usr/lib). If I move the presents to /usr/lib (noarch) the plugin no longer finds them (I was in error when I split out the presets as noarch before in comment 3).

Having said that is there really any point in splitting them into a separate package? As one package they total 1.4M which is still small.
Comment 12 Zbigniew Jędrzejewski-Szmek 2013-10-26 12:00:28 EDT
1.4 MB (the presets are bit less than 1MB i think) is borderline. If you think that it complicates things unnecessarily, then let's leave it as is.
Comment 13 Brendan Jones 2013-10-26 14:24:03 EDT
OK. I will monitor the package with each subsequent release, if the presets become unwieldy I'll split them out.

Appreciate the review! Thanks

New Package SCM Request
=======================
Package Name: lv2-fabla
Short Description: An LV2 sampler plugin
Owners: bsjones
Branches: f18 f19 f20
InitialCC:
Comment 14 Gwyn Ciesla 2013-10-28 08:06:10 EDT
Git done (by process-git-requests).
Comment 15 Fedora Update System 2013-10-28 11:47:02 EDT
lv2-fabla-1.1-1.3.20131003git5f2cb26.fc19 has been submitted as an update for Fedora 19.
https://admin.fedoraproject.org/updates/lv2-fabla-1.1-1.3.20131003git5f2cb26.fc19
Comment 16 Fedora Update System 2013-10-28 11:47:12 EDT
lv2-fabla-1.1-1.3.20131003git5f2cb26.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/lv2-fabla-1.1-1.3.20131003git5f2cb26.fc20
Comment 17 Fedora Update System 2013-10-28 15:19:41 EDT
lv2-fabla-1.1-1.3.20131003git5f2cb26.fc20 has been pushed to the Fedora 20 testing repository.
Comment 18 Fedora Update System 2013-11-10 02:06:21 EST
lv2-fabla-1.1-1.3.20131003git5f2cb26.fc20 has been pushed to the Fedora 20 stable repository.
Comment 19 Fedora Update System 2014-01-24 02:43:24 EST
lv2-fabla-1.1-1.3.20131003git5f2cb26.fc19 has been pushed to the Fedora 19 stable repository.

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