Spec URL: http://www.sentolavita.com/pkgs/setBfree.spec
SRPM URL: http://www.sentolavita.com/pkgs/setBfree-0.8.2-2.fc24.src.rpm

Updated spec and srpm to correct some lint warnings.

Link to successful koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=15789437

The spec file looks simple enough to me, but I'm not an experienced reviewer.

The koji build works for me.  (I noticed what looks like a minor bug: even when starting with "midi=alsa" its connections only showed up in the "midi" pane of qjackctl's connections window, so I needed a2jmidid to play the organ from my midi controller.  Other than that, I didn't see any obvious problems.)

(In reply to J. Bruce Fields from comment #2) 
> The koji build works for me.  (I noticed what looks like a minor bug: even
> when starting with "midi=alsa"

Sorry, I meant "midi.driver=alsa".

(In reply to J. Bruce Fields from comment #2)
> The spec file looks simple enough to me, but I'm not an experienced reviewer.
> 
> The koji build works for me.  (I noticed what looks like a minor bug: even
> when starting with "midi=alsa" its connections only showed up in the "midi"
> pane of qjackctl's connections window, so I needed a2jmidid to play the
> organ from my midi controller.  Other than that, I didn't see any obvious
> problems.)

alsa-sequencer support has been deprecated upstream, as reported in the README.md file:

Since version 0.8, alsa-sequencer support is deprecated and no longer enabled by default (even if libasound is found), It is still available via ENABLE_ALSA=yes

I think audio developers are trying to support Jack MIDI support and rely on a2jmidid to provide bridging between also and JACK. And I think that JACK 1 has got it bundled.

(In reply to Guido Aulisi from comment #4)
> alsa-sequencer support has been deprecated upstream, as reported in the
> README.md file:
> 
> Since version 0.8, alsa-sequencer support is deprecated and no longer
> enabled by default (even if libasound is found), It is still available via
> ENABLE_ALSA=yes
> 
> I think audio developers are trying to support Jack MIDI support and rely on
> a2jmidid to provide bridging between also and JACK. And I think that JACK 1
> has got it bundled.

Thanks, got it.  The documentation probably needs updating in that case (and possibly it should be warning at runtime about ignored parameters).

Anyway, minor nit, the package looks good to me.

Hi Guido,

Looking at this SPEC file, there are few things that should be improved in my opinion before do a "proper" review.

1. Please send upstream all patches and file addition that can be sent upstream. Fedora policy is to stay close to upstream. Examples of patches that should be definitely sent upstream are the one to fix the FSF address as well as fix version number (probably they forgot to change it during the release process). Also, adding the desktop/appdata files to upstream will probably be welcome as well.

2. the lv2-setBfree-plugins package does not depend on setBfree, which seems odd to me.

3. The setBfree-common package only contains license and doc. Probably is better to put those files in the setBfree package and drop the setBfree-common package. If the lv2-setBfree-plugins package does not depend on the setBfree package, it will need to carry it's own license file, otherwise the setBfree one will be enough

If you have any doubts, please do contact me here, via email, or via irc :).

> 3.

Glad you've mentioned that. It's really odd to create a tiny subpackage (plus explicit dependencies!) for those few files only. Also note that this would inherit paths from the subpackage %name and store the %license file in /usr/share/licenses/setBfree-common/ instead of /usr/share/licenses/setBfree/ (and something similar for %doc).


> Requires:       lv2 >= 1.8.1

Preferably, that one would be arch-specific due to the lv2 base package being multilib.

https://fedoraproject.org/wiki/Packaging:Guidelines#Requiring_Base_Package

Hi Fabio and  Michael
thanks for your suggestions.

I will try to explain and address the things you reported:

1. I made a pull request on github to correct FSF address and version number and it has been accepted, so the next version will be ok. I didn't make a PR for the lv2 path, because there is a conflict between Fedora guidelines for 64 bit lv2 libraries path (/usr/lib64/lv2) and the lv2 spec (PREFIX/lib/lv2), see http://lv2plug.in/pages/filesystem-hierarchy-standard.html, and I know that this change would be rejected, so I think it's better to keep the Makefile patch.

2. The setBfree and lv2-setBfree-plugins are independent, the first one is the standalone version, it works on top of the Jack audio server; the second one is the LV2 plugin version, it needs an LV2 host like ardour (http://ardour.org). You can install one of the two or both as you like.

3. For the reasons in 2, I made the common package, which contains the common files, doc and license. It's installed as a dependency when you install one or both of the main packages.

> Requires:       lv2 >= 1.8.1
Should I remove minimum version? I always see this Require in lv2 plugin packages, see for exmple http://pkgs.fedoraproject.org/cgit/rpms/lv2-EQ10Q-plugins.git/plain/lv2-EQ10Q-plugins.spec

Sorry for the delayed answer, I've been around for conferences

1a. When your patch has been submitted upstream, please, add a comment in the SPEC file with the link, so that is easy to track it. This is also very useful so that when you are going to package the version that already has the patch, and your patch will not apply cleanly you can just copy&paste the link instead of searching for it or having to remember the status of all patches you send upstream.

1b. Also, all patches that are not Fedora-specific should be upstreamed

2. Thanks for the clarification

3. Please, do not ship the -common package and put those files on both packages you are creating

Spec URL: http://www.sentolavita.com/pkgs/setBfree.spec
SRPM URL: http://www.sentolavita.com/pkgs/setBfree-0.8.2-3.fc24.src.rpm

Removed the -common package.
Added comments with links to submitted patches.

Link to successful koji build: http://koji.fedoraproject.org/koji/taskinfo?taskID=16273489

Sorry for the delay, but somehow I have not noticed the email from bugzilla of your message.
Thanks to this change, the package is good :).

Only thing to fix: add a comment on Patch0.

Now, the package is approved, but please pick 1 package to review from the pile and review it as for https://fedoraproject.org/wiki/Package_Review_Process#Reviewer
You will not be able to approve the package, but as soon as you think the package should be approved, I'll review your review and if everything is ok, I'll approve you in the Packager group so that you can push this package in Fedora and approve the review.

PS: If you sent a message and have not received any reply from my side, ping me with an email :).

You have demonstrated to understand the process.
You have not completed any review (or better, you did the review, but you did not completed the review process) but it was not your fault.
I'm going to sponsor you now, and please go forward with the good work and the review you started :).

Package request has been approved: https://admin.fedoraproject.org/pkgdb/package/rpms/setBfree

setBfree-0.8.2-3.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2016-0fcc2779ad

setBfree-0.8.2-3.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-0fcc2779ad

setBfree-0.8.2-4.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2016-c2bb71d4f0

setBfree-0.8.2-4.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2016-c2bb71d4f0

setBfree-0.8.4-1.fc25 has been submitted as an update to Fedora 25. https://bodhi.fedoraproject.org/updates/FEDORA-2017-7fbf493f41

setBfree-0.8.4-1.fc25 has been pushed to the Fedora 25 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2017-7fbf493f41

setBfree-0.8.4-1.fc25 has been pushed to the Fedora 25 stable repository. If problems still persist, please make note of it in this bug report.