diff options
| -rw-r--r-- | contributing.md | 467 |
1 files changed, 259 insertions, 208 deletions
diff --git a/contributing.md b/contributing.md index 4c347bb..38a83fd 100644 --- a/contributing.md +++ b/contributing.md @@ -11,351 +11,402 @@ `name/feature` (i.e. `loek/dll-so-poc` or `jaro/class2`) - The master branch is considered stable, and should always contain a working/compiling version of the project - - TODO: tagging / versions - # Code style +- Formatting nitty-gritty is handled by clang-format/clang-tidy (run `make + format` in the root folder of this repository to format all sources files) - ASCII only + <table><tr><th>Good</th><th>Bad</th></tr><tr><td> ```cpp - // Good + // crepe startup message std::string message = "Hello, world!"; - - // Bad - std::string message = "こんにちは世界"; ``` + </td><td> + ```cpp + // crêpe startup message + std::string message = "こんにちは世界"; + ``` + </td></tr></table> - Class names are always singular + <table><tr><th>Good</th><th>Bad</th></tr><tr><td> ```cpp - // Good - class Car {}; - - // Bad - class Cars {}; + class Foo {}; ``` - -- Explanatory comments are placed above the line(s) they are explaining + </td><td> ```cpp - // Good - // This function adds two numbers - int add(int a, int b) { - return a + b; - } - - // Bad - int add(int a, int b) { - return a + b; // This function adds two numbers - } + class Cars {}; ``` - -- Source files should only contain comments that plainly state what the code is supposed to do + </td></tr></table> +- Source files contain the following types of comments: + - What is the code supposed to do (optional) + - Implementation details (if applicable) +- Header files contain the following types of comments: + - Usage documentation (required) + - Implementation details (if they affect the header) + - Design/data structure decisions (if applicable) +- Comments are placed *above* the line(s) they are explaining + <table><tr><th>Good</th><th>Bad</th></tr><tr><td> ```cpp - // Good - // Initialize the engine - engine.init(); - - // Bad - // Initialize the engine with default settings and prepare it for running - engine.init(); + int add(int a, int b) { + // add numbers + int out = a + b; + return out; + } ``` - -- Explanatory comments in headers may be used to clarify implementation design decisions + </td><td> ```cpp - // Good - // This class handles the rendering process - class Renderer { - // This method initializes the rendering context - void init_context(); - }; - - // Bad - class Renderer { - void init_context(); // This method initializes the rendering context - }; + int add(int a, int b) { + int out = a + b; // add numbers + return out; + } ``` - -- Formatting nitty-gritty is handled by clang-format/clang-tidy (run `make format` in the root folder of this repository to format all sources files) - - -- Header includes are split into paragraphs separated by a blank line. The order is: + </td></tr></table> +- Header includes are split into paragraphs separated by a blank line. The + order is: 1. system headers (using `<`brackets`>`) 2. relative headers NOT in the same folder as the current file 3. relative headers in the same folder as the current file - ```cpp - // Good - #include <iostream> - - #include "utils/helper.h" - - #include "main.h" - - // Bad - #include "main.h" - #include <iostream> - #include "utils/helper.h" - ``` + > [!NOTE] + > When using libraries of which the header include order is important, make + > sure to separate the include statements using a blank line (clang-format + > may sort include statements, but does not sort across empty lines). -- When using libraries of which the header include order is important, make sure to separate the include statements using a blank line (clang-format may sort include statements, but does not sort across empty lines). + <table><tr><th>Good</th><th>Bad</th></tr><tr><td> ```cpp - // Good + #include <SDL2/SDL.h> #include <iostream> - - #include <boost/algorithm/string.hpp> - - // Bad - #include <iostream> - #include <boost/algorithm/string.hpp> - ``` -- All engine-related code is implemented under the `crepe` namespace, user-facing APIs under `crepe::api` (the folder structure should also reflect this). + #include "api/Sprite.h" + #include "util/log.h" + #include "SDLContext.h" + ``` + </td><td> + ```cpp + #include <SDL2/SDL.h> + #include "SDLContext.h" + #include "util/log.h" + #include <iostream> + #include "api/Sprite.h" + ``` + </td></tr></table> - `using namespace` may not be used in header files, only in source files. + <table><tr><th>Good</th><th>Bad</th></tr><tr><td> + example.h: ```cpp - // Good - // header.h namespace crepe { - void init(); + void foo(); } + ``` - // source.cpp - #include "header.h" - using namespace crepe; - void init() {} - - // Bad - // header.h + example.cpp: + ```cpp + #include "example.h" using namespace crepe; - void init(); + void foo() {} ``` + </td><td> -- Do not (indirectly) include private *dependency* headers in API header files, as these are no longer accessible when the engine is installed - + example.h: ```cpp - // Good - // api.h - namespace crepe::api { - void start(); + namespace crepe { + template <typename T> + T foo(); } + ``` - // Bad - // api.h - #include "private_dependency.h" - namespace crepe::api { - void start(); - } + example.hpp: + ```cpp + #include "example.h" + using namespace crepe; + template <typename T> + T foo(); ``` + </td></tr></table> - Getter and setter functions are appropriately prefixed with `get_` and `set_`. + <table><tr><th>Good</th><th>Bad</th></tr><tr><td> ```cpp - // Good - class Car { + class Foo { public: - int get_speed() const; - void set_speed(int speed); + int get_speed() const; + void set_speed(int speed); private: - int speed; + int speed; }; - // Bad - class Car { + ``` + </td><td> + + ```cpp + class Foo { public: - int speed() const; - void speed(int speed); + int speed() const; + void set_speed(int speed); private: - int speed; + int speed; }; ``` + </td></tr></table> -- Doxygen commands are used with a backslash instead of an at-sign (i.e. `\brief` instead of `@brief`) +- A singleton's instance is always accessed using a getter function that + instantiates its own class as a static variable within the getter function + scope, instead of storing the instance as a member variable directly: + <table><tr><th>Good</th><th>Bad</th></tr><tr><td> ```cpp - // Good - /// \brief This function adds two numbers - int add(int a, int b); - - // Bad - /// @brief This function adds two numbers - int add(int a, int b); + class Foo { + Foo & get_instance() { + static Foo instance; + return instance; + } + }; ``` - -- A singleton's instance is always accessed using a getter function that instantiates its own class as a static variable within the getter function scope, instead of storing the instance as a member variable directly: + </td><td> ```cpp - class Bad { - static Bad instance; - Bad & get_instance() { return instance; } - }; + Foo Foo::instance {}; - class Good { - Good & get_instance() { - static Good instance; - return instance; - } + class Foo { + static Foo instance; + Foo & get_instance() { return Foo::instance; } }; + ``` + </td></tr></table> -- Member variable default values should be directly defined in the class declaration instead of using the constructor. +- Member variable default values should be directly defined in the class/struct + declaration instead of using the constructor. + <table><tr><th>Good</th><th>Bad</th></tr><tr><td> ```cpp - // Good - class Car { - private: - int speed = 0; + class Foo { + int speed = 0; }; - // Bad - class Car { - private: - int speed; - Car() : speed(0) {} - }; ``` - -- Header files declare either a single class or symbols within a single namespace. + </td><td> ```cpp - // Good - // car.h - namespace crepe { - class Car {}; - } - - // Bad - // car.h - namespace crepe { - class Car {}; - class Engine {}; - } + class Foo { + Foo() : speed(0) {} + int speed; + }; ``` - -- Use of the `auto` type is not allowed, with the following exceptions: - - When naming the item type in a range-based for loop - - When calling template factory methods that explicitly name the return type in the function call signature + </td></tr></table> + +- Use of the `auto` type is *not* allowed, with the following exceptions: + - When naming the item type in a range-based for loop: + + ```cpp + for (auto & item : foo()) { + // ... + } + ``` + - When calling template factory methods that explicitly name the return type + in the function call signature + ```cpp + auto ptr = make_unique<Foo>(); + ``` - When fetching a singleton instance - - ```cpp - // Good - for (auto item : items) {} - - auto instance = Singleton::get_instance(); - - // Bad - auto speed = car.get_speed(); - ``` + ```cpp + auto & mgr = crepe::api::Config::get_instance(); + ``` - Only use member initializer lists for non-trivial types. + <table><tr><th>Good</th><th>Bad</th></tr><tr><td> ```cpp - // Good - class Car { + class Foo { public: - Car() : engine("V8") {} + Foo() : bar("baz") {} private: - std::string engine; + std::string bar; }; - // Bad - class Car { + ``` + </td><td> + + ```cpp + class Foo { public: - Car() : speed(0) {} + Foo() : bar(0) {} private: - int speed; + int bar; }; ``` + </td></tr></table> - C++-style structs should define default values for all non-trivial fields. + <table><tr><th>Good</th><th>Bad</th></tr><tr><td> ```cpp - // Good - struct Car { - std::string model = "Unknown"; + struct Foo { + int bar; + std::string baz; }; - - // Bad - struct Car { - std::string model; - Car() : model("Unknown") {} + ``` + </td><td> + + ```cpp + struct Foo { + int bar = 0; + std::string baz; }; ``` + </td></tr></table> -- Declare incomplete classes instead of including the relevant header where possible (i.e. if you only need a reference or pointer type). +- Declare incomplete classes instead of including the relevant header where + possible (i.e. if you only need a reference or raw pointer). + <table><tr><th>Good</th><th>Bad</th></tr><tr><td> ```cpp - // Good - class Engine; - class Car { - Engine* engine; + class Bar; + class Foo { + Bar & bar; }; - // Bad - #include "engine.h" - class Car { - Engine* engine; + ``` + </td><td> + + ```cpp + #include "Bar.h" + class Foo { + Bar & bar; }; ``` + </td></tr></table> -- Template functions are only declared in a `.h` header, and defined in a matching `.hpp` header. +- Template functions are only *declared* in a `.h` header, and *defined* in a + matching `.hpp` header. + <table><tr><th>Good</th><th>Bad</th></tr><tr><td> + add.h: ```cpp - // Good - // add.h template <typename T> T add(T a, T b); + + #include "add.hpp" + ``` - // add.hpp + add.hpp: + ```cpp #include "add.h" + template <typename T> T add(T a, T b) { - return a + b; + return a + b; } - - // Bad - // add.h + ``` + </td><td> + + add.h: + ```cpp template <typename T> T add(T a, T b) { - return a + b; + return a + b; } ``` + </td></tr></table> -- Where possible, end (initializer) lists with a trailing comma (e.g. with structs, enums) +- Where possible, end (initializer) lists with a trailing comma (e.g. with + structs, enums) + <table><tr><th>Good</th><th>Bad</th></tr><tr><td> ```cpp - // Good enum Color { - Red, - Green, - Blue, + Red, + Green, + Blue, }; - // Bad + ``` + </td><td> + + ```cpp enum Color { - Red, - Green, - Blue + Red, + Green, + Blue }; ``` + </td></tr></table> +- `#pragma` should be used instead of include guards + <table><tr><th>Good</th><th>Bad</th></tr><tr><td> -## CMakeLists specific + ```cpp + #pragma once + + // ... + ``` + </td><td> + + ```cpp + #ifndef __INCLUDED_H + #define __INCLUDED_H + + // ... + + #endif + ``` + </td></tr></table> + +## CMakeLists-specific - Make sure list arguments (e.g. sources, libraries) given to commands (e.g. `target_sources`, `target_link_libraries`) are on separate lines. This makes resolving merge conflicts when multiple sources were added by different people to the same CMakeLists.txt easier. +# Structure + +- All engine-related code is implemented under the `crepe` namespace, + user-facing APIs under `crepe::api` (the folder structure should also reflect + this). +- Do not (indirectly) include private *dependency* headers in API header files, + as these are no longer accessible when the engine is installed +- Header files declare either a single class or symbols within a single + namespace. +- TODO: folder structure + # Documentation - All documentation is written in U.S. English +- Doxygen commands are used with a backslash instead of an at-sign. + <table><tr><th>Good</th><th>Bad</th></tr><tr><td> + + ```cpp + /** + * \brief do something + * + * \param bar Magic number + */ + void foo(int bar); + ``` + </td><td> + + ```cpp + /** + * @brief do something + * + * @param bar Magic number + */ + void foo(); + ``` + </td></tr></table> # Libraries |