Hi Everyone, This can hardly be called a review, as I didn't look thoroughly at the implementation. (I only checked what happens on the resize() over the capacity(), as this was not clear from the docs.) I think that ultimately the library would be a useful addition to Boost, however I would not like to rush it before its design is fully baked, and my impression is that it isn't yet. Therefore my recommendation would be to REJECT the library at this point, and encourage the authors to resubmit it for the second review once the design goals and design decisions have been clearly settled and communicated. My vision of a review is that we: 1. First evaluate the design goals, whether they are worth pursuing 2. Then we evaluate the design *against* those goals. 3. Then we evaluate the documentation and implementation *against* the design and goals. I cannot even go to #3, as to me #1 is missing some parts. There are different possible goals: 1. Treat this new string as many other STL sequence containers with variable size. In particular, users use it as if its max_size() were infinite, and are prepared that at some point an exception might be thrown. 2. Treat it as a "drop-in replacement" for std::string with no-heap-alloc guarantee. 3. Make something suitable for embedded systems where the user provides the guarantee: "I will never make the size() of this string go above this value. If I did, it would be plain clear that the program is not doing what it was supposed to." These goal 3 is in clear conflict with either 1 or 2. This becomes clear when we consider what should be the response to doing a resize() over capacity(): a precondition violation or a normal course of action signaled by an exception. The authors should make a choice which of these goals is not supported and communicate it clearly from the outset in the docs. I think this decision has already been made: the library was extracted from Boost.Beast and there the purpose must have been clear. It is just that it is not communicated in fixed_string library. Any decision is good. In C++ performance trade-offs are part of the contract (interface), so there is room in Boost for many strings: small strings, short codes, fixed size strings, fixed capacity strings. But for each library it should be clear what trade-offs were chosen and why. We have a situation that there exists static_vactor in Boost, which defines the resize() over capacity() to be a precondition violation. And fixed_string needs to provide some response to this fact: why did you choose a different design? I consider the choice of name to be part of the design: you cannot assign the name before you have decided on the goals. "static_string" might be a good name if you chose the same trade-offs as static_vector. But if you make different trade-offs, it might be better to use a different name. Making a resize() over capacity() a precondition violation is a *feature* useful for bug detection and I do not consider it a valid argument that "library will throw exceptions and if you never resize() over capacity() you will never see exceptions or std::abort()". If this is a precondition, then I expect of a library to put some BOOST_ASSERT() or _builtin_unreachable() in those paths to enable better bug detection. I find the goal "a drop-in replacement for std::string" invalid. You surely mean only a drop in replacement for some subset of usages. I guess this subset comes from the usage in Beast, and I would expect that it is clearly outlined what this subset is. One of the operations on strings that I use most often is operator+, so when I hear that this operation is not implemented in a class that claims to be a drop-in replacement for string, I see a contradiction. There might be valid reasons not to provide operator+, but the documentation cannot state that this is a drop-in replacement for std::string. I think that only after the above issues have been sorted out can we have a review of the implementation and the documentation. One remark about documentation is that I was not able to find what `resize()` over `capacity()` does. This is the most important information that I was after. anything else is more less intuitively clear from the analogy to std::string. I hope that this feedback doesn't sound discouraging. I appreciate the work Krystian and Vinnie have put into making this submission, and I would like to see this library ultimately included in Boost. Regards, &rzej;