diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -57,6 +57,7 @@ Here are some handy links for development practices aligned with Bitcoin ABC: +- [Developer Notes](doc/developer-notes.md) - [Statement of Bitcoin ABC Values and Visions](https://www.yours.org/content/bitcoin-abc---our-values-and-vision-a282afaade7c) - How to Do Code Reviews Like a Human [Part 1](https://mtlynch.io/human-code-reviews-1/) [Part 2](https://mtlynch.io/human-code-reviews-2/) - [Large Diffs Are Hurting Your Ability To Ship](https://medium.com/@kurtisnusbaum/large-diffs-are-hurting-your-ability-to-ship-e0b2b41e8acf) diff --git a/doc/developer-notes.md b/doc/developer-notes.md --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -23,18 +23,32 @@ Block style example: ```c++ -namespace foo +// namespaces should be lower_snake_case +namespace foo_bar_bob { class Class { - bool Function(const std::string& s, int n) +private: + // memberVariable's name should be lowerCamelCase, and be a noun. + int memberVariable; + +public: + /** + * The documentation before a function should follow Doxygen spec. + * The name of the function should start with an english verb which indicates + * what it is doing. It should be CamelCase. + * @param[in] s A description + * @param[in] n Another argument description + * @pre Precondition for function... + */ + bool PerformTask(const std::string& s, int n) { // Comment summarising what this section of code does for (int i = 0; i < n; ++i) { // When something fails, return early - if (!Something()) return false; + if (!IsSomething()) return false; ... - if (SomethingElse()) { + if (IsSomethingElse()) { DoMore(); } else { DoLess(); @@ -48,6 +62,10 @@ } ``` +The naming convention roughly follows [Microsoft Naming Conventions](https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/general-naming-conventions) + +C++ Coding Standards should strive to follow the [LLVM Coding Stanadrds](https://llvm.org/docs/CodingStandards.html) + Doxygen comments ----------------- @@ -228,23 +246,6 @@ A few non-style-related recommendations for developers, as well as points to pay attention to for reviewers of Bitcoin Core code. -General Bitcoin Core ----------------------- - -- New features should be exposed on RPC first, then can be made available in the GUI - - - *Rationale*: RPC allows for better automatic testing. The test suite for - the GUI is very limited - -- Make sure pull requests pass Travis CI before merging - - - *Rationale*: Makes sure that they pass thorough testing, and that the tester will keep passing - on the master branch. Otherwise all new pull requests will start failing the tests, resulting in - confusion and mayhem - - - *Explanation*: If the test suite is to be updated for a change, this has to - be done first - Wallet ------- @@ -321,11 +322,6 @@ Strings and formatting ------------------------ -- Be careful of `LogPrint` versus `LogPrintf`. `LogPrint` takes a `category` argument, `LogPrintf` does not. - - - *Rationale*: Confusion of these can result in runtime exceptions due to - formatting mismatch, and it is easy to get wrong because of subtly similar naming - - Use `std::string`, avoid C string manipulation functions - *Rationale*: C++ string handling is marginally safer, less scope for @@ -336,10 +332,6 @@ - *Rationale*: These functions do overflow checking, and avoid pesky locale issues -- For `strprintf`, `LogPrint`, `LogPrintf` formatting characters don't need size specifiers - - - *Rationale*: Bitcoin Core uses tinyformat, which is type safe. Leave them out to avoid confusion - Variable names -------------- @@ -348,24 +340,6 @@ Please name variables so that their names do not shadow variables defined in the source code. -E.g. in member initializers, prepend `_` to the argument name shadowing the -member name: - -```c++ -class AddressBookPage -{ - Mode mode; -} - -AddressBookPage::AddressBookPage(Mode _mode) : - mode(_mode) -... -``` - -When using nested cycles, do not name the inner cycle variable the same as in -upper cycle etc. - - Threads and synchronization ----------------------------