On Fri, Jun 10, 2016 at 3:43 PM, Brad King <brad.k...@kitware.com> wrote: > On 06/10/2016 09:30 AM, Daniel Pfeifer wrote: >> By looking at the CMake source code, there are some inconsistencies >> regarding coding conventions. This is not a big problem and fixing >> them probably does not have a high priority. >> I would like to know what conventions to follow for new code. > > Please look at documenting this in CONTRIBUTING.rst once we resolve > this discussion.
ok. >> Formatting: No longer an issue. A .clang-format is provided and most >> inconsistencies have disappeared. > > Yes, and thanks for all your work on that. > >> Braces around single conditional statements: This is currently not >> consistent. Should they be avoided? Should they be required? We could >> add the missing ones with clang-tidy. If we want them. > > The intention has been to use braces for all blocks even with single > statements. Due to lack of documentation and enforcement this has > not been maintained consistently. I have added some missing ones: https://cmake.org/gitweb?p=stage/cmake.git;a=commit;h=a16bf141bc5d393b27d13d8235d95a1b034052c2 >> Naming conventions: Classes are named cmLikeThis. Member functions >> and member variables are named LikeThis. Local variables are named >> likeThis. Members are always accessed with `this->`. > > Yes, though there are also many local variables named `like_this` too. > I have no strong feelings on enforcing local variable name conventions. > >> So far it is pretty consistent. But how to name free functions and >> macros? I have seen all kinds of variations. > > No convention was ever established for those. I see. Any strong opinion whether a convention should be established? I would assume UPPER_CASE for macros. Any prefix? Any opinions about free functions? >> `const T` or `T const` (also: `const T&` or `T const&`): Currently, >> CMake uses both. I have not analyzed which one dominates. > > I prefer `T const` always, except for `const char*` specifically. Good. >> Passing and returning strings: We have `const char*`, `std::string`, >> and `std::string const&`. Unifying this can affect performance. >> Storing `const char*` in a `std::string` creates a (potentially >> unneded) copy (assuming it is not null). `const char*` can distinguish >> invalid from empty. Do we actually need this distinction? > > Last year we had a few cleanup passes done along those lines, but > it is far from consistent everywhere. There are cases where we > need to distinguish invalid from empty, like GetDefinition(). It > would be nice to have an optional<string> or something like that > so we can keep the distinction but avoid using `const char*` all > over. Any change like this should be introduced gradually, as > many of the related areas are performance sensitive. So the convention would be to use `std::string` or `cmOptional<std::string>` as return values. Should `std::string const&` be allowed as return value? Arguments should be preferred as `const&` I assume. Some other questions: Can't `std::ifstream` and `std::ofstream` be used directly? It seams that kwsys does some workarounds for Visual Studio 2005 and newer. That surprises me. Workarounds are usually used for older, not newer compilers. I see that `#include <string.h>` is preferred over `#include <cstring>`. Are there old compilers supported that prohibit the latter or was it just not changed yet? cmStandardIncludes.h includes many standard headers. That is against the idea of include-what-you-use. Is there a particular requirement or was it just not cleaned up yet? Should we begin using newer language features optionally by providing macros like CM_OVERRIDE? -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers