Hi Lucas, On Wed, Jun 27, 2012 at 7:22 PM, Lucas De Marchi <lucas.demarchi@xxxxxxxxxxxxxx> wrote: >> No. According to Lucas I removed unused constants. >> What wrong with enum? Look into sap.h, vhci.h, reporter.h, parser.h, cups.h and... avrcp.c ("enum battery_status" is here) > > I strongly NACKed this before. Not only because of the unused constants. > >> >> I have idea to make enum new convention in BlueZ. See sap.h - it looks very clearly for me. Pros for used enum is that enum can be thread as group of constants, mostly constants defined in specification. But "magic constants" should stay define - mostly defined by developer. >> Disadvantage for "define" is that as you can see in BlueZ AVRCP - you must use additional comment to describe group of constants. > > I don't see any advantage in being able to group them. You still have > to define the name by prefixing it with the namespace. If it was > already enum, fine - changing for the sake of change only is not good. > >> >> Luiz, why "nak"? Could you present your point of view? > > IMO this change doesn't give us anything Exactly, enum gives us nothing, no type checking or anything, so why bother. @Michal: As for the rest of the patches, I will apply the typo and code style fixes that are straightforward, the rest will probably need rebasing. I specially liked the idea of avrcp_session, it will be really useful, I guess we can even store the avctp_session on it. As for the dummy player, my initial take was that it was necessary, but you are actually not using it as a place holder, instead it just return errors so in that I would just check if there is no player available send the proper errors. -- Luiz Augusto von Dentz -- To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html