From ff250ce08e6f2fc933372c51f2f152fbc0b9f66c Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Wed, 23 Oct 2024 13:58:28 +0100 Subject: [PATCH] Documentation improvements --- CONTRIBUTING.md | 6 ++++++ src/libxrpl/basics/contract.cpp | 6 ++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a74c9aaaf50..0609cd7f2e5 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -385,6 +385,10 @@ For this reason: function name. NOTE: the purpose of name is to provide stable means of unique identification of every contract; for this reason try to avoid elements which can change in some obvious refactors or when reinforcing the condition. +* Contract description typically (except for `UNREACHABLE`) should describe the + _expected_ condition, as in "I assert that _expected_ is true". +* Contract description for `UNREACHABLE` should describe the _unexpected_ + situation which caused the line to have been reached. * Example good name for an `UNREACHABLE` macro `"Json::operator==(Value, Value) : invalid type"`; example good name for an `ASSERT` macro `"Json::Value::asCString : valid type"`. @@ -392,6 +396,8 @@ For this reason: `"RFC1751::insert(char* s, int x, int start, int length) : length is greater than or equal zero"` (missing namespace, unnecessary full function signature, description too verbose). Good name: `"ripple::RFC1751::insert : minimum length"`. +* In **few** well-justified cases a non-standard name can be used, in which case a + comment should be placed to explain the rationale (example in `contract.cpp`) * Do **not** rename a contract without a good reason (e.g. the name no longer reflects the location or the condition being checked) * Do not use `std::unreachable` diff --git a/src/libxrpl/basics/contract.cpp b/src/libxrpl/basics/contract.cpp index b26058be537..4f0a74644af 100644 --- a/src/libxrpl/basics/contract.cpp +++ b/src/libxrpl/basics/contract.cpp @@ -49,9 +49,11 @@ LogicError(std::string const& s) noexcept { JLOG(debugLog().fatal()) << s; std::cerr << "Logic error: " << s << std::endl; - // Use a non-standard contract naming here (without namespace), because + // Use a non-standard contract naming here (without namespace) because // it's the only location where various unrelated execution paths may - // register an error; this is also why "message" parameter is passed here. + // register an error; this is also why the "message" parameter is passed + // here. + // For the above reasons, we want this contract to stand out. UNREACHABLE("LogicError", {{"message", s}}); detail::accessViolation(); }