[pulseaudio-discuss] Revisiting raop2 patches

[Hajime Fujita]

Corin, Tanu, and Anton,

Thank you folks for your responses!

> On Oct 24, 2016, at 4:05 PM, Anton Lundin <glance at acc.umu.se> wrote:
> 
> On 24 October, 2016 - Colin Leroy wrote:
> 
>> On 24 October 2016 at 20h58, Tanu Kaskinen wrote:
>> 
>> Hi, 
>> 
>>> It would be great to have more people reviewing patches, but I don't
>>> know how to acquire those people. I don't think the reviews have to
>>> necessarily be done by someone with a title of "maintainer", but on
>>> the other hand, giving much trust to drive-by contributors seems risky
>>> too...
>> 
>> Reviewing is hard when you don't have an extensive knowledge of the
>> codebase... I'd propose to do it, but I'd suck at it - "yeah, seems
>> fine to me" :D
>> 
> 
> I think we can file the raop2 code under a quite special category. It
> doesn't have a big inpact outside of the raop2 code, and those bits are
> quite trivial to review. I just read them my self and they looked ok to
> me =)

That sounds interesting. Not sure if it’s a good idea to essentially “skipping" reviews for non-raop2 patches, but it makes a lot of sense to focus on the patches that touch the core and other modules.

Actually as an author of the patches my expectation for the review was to make sure
a) we didn’t screw up when making changes to other parts of PA
b) naming conventions are acceptable
c) pulsecore API usages are correct

Any other concerns from maintainer’s point of view? I think this is also a good opportunity for me (and potentially others) to learn about the review process.

> On the other hand, the raop2 code itself is quit the opposite. It
> requires understanding of raop2 and pa.
> 
> Here comes the point: The current raop2 code is pretty much unusable.
> There are probably very few who have such old devices that they work
> with that code, and the changes only affects raop2 users.

This is a very valid point. I’d say this was already true three years ago when I started working on this project. And it was so disappointing to know that module-raop-sink didn’t work with most of the raop devices in the market despite its name. That’s the why I started working on this.

> I'd love to see the code merged, and even with the
> module-raop-discover module disabled by default. That way this change
> only affects users who really wants and uses this feature, and probably
> need this code to get their gear working.

FYI: in my understanding it’s already disabled by default.

> So my suggestion is to take a look at the non-raop2 patches so the
> maintainers are happy with those, and merge the lot.
> 
> 
> //Anton
> 
> 
> -- 
> Anton Lundin	+46702-161604
> _______________________________________________
> pulseaudio-discuss mailing list
> pulseaudio-discuss at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss