Bug 1006187
| Summary: | Review Request: lv2-fabla - an LV2 sampler plugin | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Brendan Jones <brendan.jones.it> |
| Component: | Package Review | Assignee: | Zbigniew Jędrzejewski-Szmek <zbyszek> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | unspecified | Docs Contact: | |
| Priority: | unspecified | ||
| Version: | rawhide | CC: | notting, zbyszek |
| Target Milestone: | --- | Flags: | zbyszek:
fedora-review+
gwync: fedora-cvs+ |
| Target Release: | --- | ||
| Hardware: | Unspecified | ||
| OS: | Unspecified | ||
| Whiteboard: | |||
| 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 07:06:21 UTC | Type: | Bug |
| Regression: | --- | Mount Type: | --- |
| Documentation: | --- | CRM: | |
| Verified Versions: | Category: | --- | |
| oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
| Cloudforms Team: | --- | Target Upstream Version: | |
| Embargoed: | |||
| Bug Depends On: | |||
| Bug Blocks: | 805236 | ||
|
Description
Brendan Jones
2013-09-10 07:47:38 UTC
New upstream source, standardize github URLs. SPEC: http://bsjones.fedorapeople.org/reviews/lv2-fabla.spec SRPM: http://bsjones.fedorapeople.org/reviews/lv2-fabla-1.1-0.1.20130913git0122750.fc19.src.rpm 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'. 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 > 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.
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. PS. please redownload the SRPM, may have been uploading again while you were looking 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 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. 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 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. 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. 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. 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: Git done (by process-git-requests). 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 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 lv2-fabla-1.1-1.3.20131003git5f2cb26.fc20 has been pushed to the Fedora 20 testing repository. lv2-fabla-1.1-1.3.20131003git5f2cb26.fc20 has been pushed to the Fedora 20 stable repository. lv2-fabla-1.1-1.3.20131003git5f2cb26.fc19 has been pushed to the Fedora 19 stable repository. |