
AMDG I started my review, but I don't think I'm going to get it finished, so I'm throwing in a partial review as is: HOME: - "View this code in Github" This is fine for online viewing, but for a packaged release it's better to link the local source. - "contains an error information" "information" shouldn't have an indefinite article. PREREQUISITES: - "It is worth turning on C++ 17 if you can, there are..." comma splice. BUILD AND INSTALL: This section refers to all kinds of things that do not exist in the review version. Such as: - CMake - Single header version - outcome/include/outcome.hpp DECISION MATRIX: - The font size in the decision matrix scales with the page size, which may make it unreadable. (I had to zoom to 300%, before I could read the third flow chart) It's also completely unreadable without javascript. TUTORIAL: RESULT<T, EC>: - OUTCOME_V2_NAMSPACE. This seems singularly pointless. How does accessing the namespace via a macro help with backwards compatiblity? It will break source compatibility exactly as much as if you just used a normal namespace. If you're concerned with binary compatibility, then the only thing that matters is the symbol mangling and there's no need to expose this ugliness to the user. Also, why is this attached to result, and not in its own section. - If some type `X` is both convertible to `T` and `EC` parallel structure: 'convertible' should either be placed before 'both' or be duplicated on both sides of the 'and.' - You say that it's ambiguous if the constructor argument is convertible to both T and EC. Is it still ambiguous if it is exactly T or EC? If so, that seems wrong. If not, you need to explain the behavior more clearly. - Why do you need to use tagged constructors instead of just casting? (I suppose the fact that you felt the need to have these indicates to me that an exact match /is/ ambiguous.) tutorial/result/inspecting: - "Suppose we will be writing function..." function needs an article. - "integral number" This is just a verbose way of saying "integer." - "Type result<void>" -> "The type result<void>" - "Class template `result<>` is declared with attribute [[nodiscard]] which means compiler" Add "the" before "class," "attribute," and "compiler." - "In the implementation we will use function `convert`" "*the* function" - "#2. Function `.value()` extracts the successfully returned `BigInt`." "*The* function". Also it's an int, not a BigInt. - I noticed that the starts of the list items after the #N. are not vertically aligned, which looks a little odd for #1, #2, #3, and #4, which are short enough to make it obvious. This appears to be because each item is treated as a normal paragragh which happens to begin with a number instead of a proper list item. - "It implies that the function call in the second argument" I presume that it takes a generic expression, not just a function call. - "It is defined as" The antecedent of "It" is ambiguous. It refers to the function in the preceding sentence, but when I first read it I assumed that it referred to "OUTCOME_TRY" - I think the name OUTCOME_TRY is misleading because it has the exact opposite meaning of "try." try/catch stops stack unwinding, but OUTCOME_TRY is used to implement manual stack unwinding. - I'm not sure what I think of as_failure. I'd actually prefer to just write return r; - "this will be covered later" A link would be nice. - I can't see the point of having success<void> default construct T. It seems like dangerous implicit behavior, unless you make it so that success is variadic and constructs the result in place. Also, there is no type named success. It's called success_type. tutorial/result/try - OUTCOME_TRYV I think it would be nicer if you used a single macro that has different behavior depending on whether it is passed 1 or 2 arguments. It would be even better if you could also merge OUTCOME_TRYX in. The behavior of OUTCOME_TRYV is logically the same as that of OUTCOME_TRYX for a result<void>. tutorial/outcome: tutorial/outcome/inspecting: - "outcome<> has also function" -> "outcome<> also has a function" - How does outcome::value interact with custom error_code type? It needs to know what exception type to throw, no? tutorial/payload: - "...symbolising that into human readable text at the moment of conversion into human readable text" This sentence does not parse. - "#4 Upon a namespace-localised `result` from library A being copy/moved into a namespace-localised `result` from C bindings..." I can't understand this at all. What is a "namespace-localised result" How does this relate to a custom payload? A plain error_code has everything you need to set errno. general: - In code examples, _'s can sometimes disappear mysteriously. I haven't figured out the exact conditions, but resizing the window changes the result. Seems to be a result of overlapping with the line underneath. - Also, the lines are too long. They still wrap even when I put the browser in full screen. (Admittedly, the VM is only 1280x768) In Christ, Steven Watanabe