<div dir="ltr"><div>On Fri, 15 Jul 2016 12:15:21 -0400 Felipe Sateler <<a href="mailto:fsateler@debian.org">fsateler@debian.org</a>> wrote:<br>> On 15 July 2016 at 08:56, Willem Mulder <<a href="mailto:14mrh4x0r@gmail.com">14mrh4x0r@gmail.com</a>> wrote:<br>> > [...]<br>> <br>> Thanks! :)<br>> <br>> > I based the patch on the package's git repo. I'd also like some feedback<br>> > on it, since this is the first time I actually did something like this.<br>> <br>> The patch looks good, but there are some questions that need to be<br>> resolved first:<br>> <br>> 1. Is the equalizer sink useful without qpaeq?<br>> 2. Do users actually want the module, or the equalizer?<br>> <br>> As I understand it, the answer to 1 is: not at the moment, qpaeq is<br>> the only available client; this makes the answer to 2 "no". Therefore,<br>> I think the new package should be called qpaeq, and describe the<br>> equalizer application.<br><br></div>My view of this is that users will be looking for 'an' equalizer, not for qpaeq per se. Looking at other distributions, I see the package is called 'pulseaudio-equalizer' (Fedora, Arch Linux, openSUSE). Perhaps we could go with that name, if only for the sake of uniformity?<br><div><div><div><br>> Other than that, it looks good. The only unfortunate thing is that it<br>> won't work by default as the dbus module is not loaded in the default<br>> configuration. But fixing this would mean patching upstream code,<br>> which is more work (and I don't think it should block actually<br>> shipping the app).<br><br></div><div>qpaeq itself says to enable the module if it is not, so I think it's safe to ignore this.<br></div><div><br>> Did you test that the (python) dependencies you added are enough to<br>> run the app? Ie, install on a clean system/chroot and that it starts?<br><br></div><div>I have not tested it through a chroot, although I have tested it on a live environment. I've added dependencies on the Debian packages providing the explicitly imported Python packages, so it should be safe (if it's not, that's a bug in the packages concerned).<br></div><div><br>> -- <br>> <br>> Saludos,<br>> Felipe Sateler<br><br></div><div>Kind regards,<br><br></div><div>Willem Mulder<br></div></div></div></div>