Hi Lukáš, In the specific case of the streaming agent, I believe it matters for instant productivity that the code follow a style that does not require additional thinking on Frediano’s part. So if Frediano likes it, it’s fine by me, otherwise don’t care. Also, rather than invent a style, I’d rather adopt an standard coding style, e.g. Google’s. And then use clang-format to enforce all the machine-enforceable parts of it. Regards, Christophe PS: Some comments on your suggestions anyway… > On 29 Jan 2018, at 15:19, Lukáš Hrázký <lhrazky@xxxxxxxxxx> wrote: > > Hello everybody, > > I'd like to discuss a few things about the coding style for C++ in > Spice (looking at the streaming agent atm). > > Trying to keep this short and concise. > > > 1. Method names > Currently the method names are in CamelCase throughout the streaming > agent. Methods are basically functions attached to a class, I suggest > we use snake_case to be consistent with the function names. > > It's rather confusing when you see a call like SomeObject(), which > looks like a constructor, but you actually find out it's a method call > from another method of the same class. Naming a method with a name that can also be a class is always ambiguous, CamelCase or not. Is color() a method or an ctor? So DeCamelCaseIfication not a solution to that problem. BTW, CamelCase is so frequent in C++ that it often can be used to identify code as being C++ as opposed to plain C. To wit: LLVM, WebKit, Qt, etc. > > > ;2. Namespace names > Although not standard (you may have different experience), usually > namespaces are lowercase in C++. By that token, so do classes (in all of the standard library). But it’s generally not true outside of the standard library. > Also, they are hierarchical, I suggest > we use that and in streaming agent we change the namespace like so: > > SpiceStreamingAgent -> spice::streamingagent > > or (imho better): > SpiceStreamingAgent -> spice::streaming_agent > > And stick to this scheme, i.e. lowercase and toplevel namespace > 'spice', inside it a namespace of the component. Not against the idea, but two levels of namespace for 2000 LOCs seems a tad bit overkill… > > > 3. Namespace coding style > > a) Let's not use `using namespace ...` ever even in .cpp files (see > i.e. [1]). In streaming agent we have at the beginning of every .cpp: > > using namespace std; > using namespace SpiceStreamingAgent; Again, 2000 lines of code, unlikely to grow much. Google’s rule applies to their mega-projects, but for the agent, I think that “using namespace” makes the code leaner. > > For namespace std, "std::" is not a long prefix, clearly expresses the > identifier is from the standard library and AFAIK most C++ projects use > it this way. > > For namespace SpiceStreamingAgent, I didn't even know it worked for > definition of symbols in the namespace. First time I see it, it is very > unusual. see b). > > b) Let's keep the following coding style for namespaces, i.e. for > streaming agent: > > namespace spice { > namespace streaming_agent { > > THE_CODE > > }} // namespace spice::streaming_agent Not too enthusiastic about }} > > > We should add the guidelines to the website next to the C coding style, > but I have no intention to be exhaustive (see [1] for how long it can > be), let's add important cases as they come up and just use common > sense, keep the style of the local code and codereview to keep things > in check? Let’s first share our preference on existing styles to see if we agree on anything… As for me, I have a slight preference for the LLVM coding style, but I made modifications in my own clang-format files. Regards Christophe > > Lukas > > > [1] https://google.github.io/styleguide/cppguide.html#Namespaces > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/spice-devel _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel