Bug 1006187 - Review Request: lv2-fabla - an LV2 sampler plugin
Summary: Review Request: lv2-fabla - an LV2 sampler plugin
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
Target Milestone: ---
Assignee: Zbigniew Jędrzejewski-Szmek
QA Contact: Fedora Extras Quality Assurance
Depends On:
Blocks: FedoraAudio
TreeView+ depends on / blocked
Reported: 2013-09-10 07:47 UTC by Brendan Jones
Modified: 2014-01-24 07:43 UTC (History)
2 users (show)

Fixed In Version: lv2-fabla-1.1-1.3.20131003git5f2cb26.fc19
Doc Type: Bug Fix
Doc Text:
Clone Of:
Last Closed: 2013-11-10 07:06:21 UTC
Type: Bug
zbyszek: fedora-review+
gwync: fedora-cvs+

Attachments (Terms of Use)

Description Brendan Jones 2013-09-10 07:47:38 UTC
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-25 00:10:16 UTC
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:

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-25 02:18:59 UTC
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-25 03:37:53 UTC
> 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-25 03:59:59 UTC
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 04:01:27 UTC
PS. please redownload the SRPM, may have been uploading again while you were looking

Comment 7 Brendan Jones 2013-10-25 04:12:06 UTC
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 23:16:56 UTC
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-26 02:48:47 UTC
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


Comment 10 Zbigniew Jędrzejewski-Szmek 2013-10-26 13:49:49 UTC
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 15:51:12 UTC

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 16:00:28 UTC
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 18:24:03 UTC
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

Comment 14 Gwyn Ciesla 2013-10-28 12:06:10 UTC
Git done (by process-git-requests).

Comment 15 Fedora Update System 2013-10-28 15:47:02 UTC
lv2-fabla-1.1-1.3.20131003git5f2cb26.fc19 has been submitted as an update for Fedora 19.

Comment 16 Fedora Update System 2013-10-28 15:47:12 UTC
lv2-fabla-1.1-1.3.20131003git5f2cb26.fc20 has been submitted as an update for Fedora 20.

Comment 17 Fedora Update System 2013-10-28 19:19:41 UTC
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 07:06:21 UTC
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 07:43:24 UTC
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.