diff options
Diffstat (limited to 'doc/developer-notes.md')
| -rw-r--r-- | doc/developer-notes.md | 51 |
1 files changed, 41 insertions, 10 deletions
diff --git a/doc/developer-notes.md b/doc/developer-notes.md index d783a7a8ae..33c6ab9cb3 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -4,7 +4,7 @@ Developer Notes Various coding styles have been used during the history of the codebase, and the result is not very consistent. However, we're now trying to converge to a single style, which is specified below. When writing patches, favor the new -style over attempting to mimick the surrounding style, except for move-only +style over attempting to mimic the surrounding style, except for move-only commits. Do not submit patches solely to modify the style of existing code. @@ -37,6 +37,8 @@ code. - **Miscellaneous** - `++i` is preferred over `i++`. + - `nullptr` is preferred over `NULL` or `(void*)0`. + - `static_assert` is preferred over `assert` where possible. Generally; compile-time checking is preferred over run-time checking. Block style example: ```c++ @@ -275,7 +277,7 @@ Wallet - *Rationale*: In RPC code that conditionally uses the wallet (such as `validateaddress`) it is easy to forget that global pointer `pwalletMain` - can be NULL. See `test/functional/disablewallet.py` for functional tests + can be nullptr. See `test/functional/disablewallet.py` for functional tests exercising the API with `-disablewallet` - Include `db_cxx.h` (BerkeleyDB header) only when `ENABLE_WALLET` is set @@ -330,6 +332,12 @@ C++ data structures - *Rationale*: Ensure determinism by avoiding accidental use of uninitialized values. Also, static analyzers balk about this. +- By default, declare single-argument constructors `explicit`. + + - *Rationale*: This is a precaution to avoid unintended conversions that might + arise when single-argument constructors are used as implicit conversion + functions. + - Use explicitly signed or unsigned `char`s, or even better `uint8_t` and `int8_t`. Do not use bare `char` unless it is to pass to a third-party API. This type can be signed or unsigned depending on the architecture, which can @@ -541,6 +549,26 @@ Git and GitHub tips or `git fetch upstream-pull`. Afterwards, you can use `upstream-pull/NUMBER/head` in arguments to `git show`, `git checkout` and anywhere a commit id would be acceptable to see the changes from pull request NUMBER. +Scripted diffs +-------------- + +For reformatting and refactoring commits where the changes can be easily automated using a bash script, we use +scripted-diff commits. The bash script is included in the commit message and our Travis CI job checks that +the result of the script is identical to the commit. This aids reviewers since they can verify that the script +does exactly what it's supposed to do. It is also helpful for rebasing (since the same script can just be re-run +on the new master commit). + +To create a scripted-diff: + +- start the commit message with `scripted-diff:` (and then a description of the diff on the same line) +- in the commit message include the bash script between lines containing just the following text: + - `-BEGIN VERIFY SCRIPT-` + - `-END VERIFY SCRIPT-` + +The scripted-diff is verified by the tool `contrib/devtools/commit-script-check.sh` + +Commit `bb81e173` is an example of a scripted-diff. + RPC interface guidelines -------------------------- @@ -570,16 +598,14 @@ A few guidelines for introducing and reviewing new RPC interfaces: is specified as-is in BIP22. - Missing arguments and 'null' should be treated the same: as default values. If there is no - default value, both cases should fail in the same way. + default value, both cases should fail in the same way. The easiest way to follow this + guideline is detect unspecified arguments with `params[x].isNull()` instead of + `params.size() <= x`. The former returns true if the argument is either null or missing, + while the latter returns true if is missing, and false if it is null. - *Rationale*: Avoids surprises when switching to name-based arguments. Missing name-based arguments are passed as 'null'. - - *Exception*: Many legacy exceptions to this exist, one of the worst ones is - `getbalance` which follows a completely different code path based on the - number of arguments. We are still in the process of cleaning these up. Do not introduce - new ones. - - Try not to overload methods on argument type. E.g. don't make `getblock(true)` and `getblock("hash")` do different things. @@ -607,9 +633,14 @@ A few guidelines for introducing and reviewing new RPC interfaces: from there. - A RPC method must either be a wallet method or a non-wallet method. Do not - introduce new methods such as `getinfo` and `signrawtransaction` that differ - in behavior based on presence of a wallet. + introduce new methods such as `signrawtransaction` that differ in behavior + based on presence of a wallet. - *Rationale*: as well as complicating the implementation and interfering with the introduction of multi-wallet, wallet and non-wallet code should be separated to avoid introducing circular dependencies between code units. + +- Try to make the RPC response a JSON object. + + - *Rationale*: If a RPC response is not a JSON object then it is harder to avoid API breakage if + new data in the response is needed. |