[libcxx-commits] [PATCH] D147473: [libc++] Remove the synopses

Mark de Wever via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Sat Apr 8 09:53:21 PDT 2023


Mordante requested changes to this revision.
Mordante added a comment.
This revision now requires changes to proceed.

In D147473#4241871 <https://reviews.llvm.org/D147473#4241871>, @philnik wrote:

> In D147473#4241835 <https://reviews.llvm.org/D147473#4241835>, @var-const wrote:
>
>> Has this been discussed previously, e.g. on Discord?
>
> This came up at https://reviews.llvm.org/D144994?id=502156#inline-1403408. Given that often nothing happens without a patch w.r.t. there kinds of changes I thought I'd create one, but I'm happy to discuss this further if there are any objections to this change.

Actually I would have preferred to discuss it first.

I'm also not too happy with the synopsis, but I really think it should be discussed. The commit message mentions that we "and add notes if a paper is not complete yet." which I noticed is not always happening. I feel we are doing that better nowadays, but this is something that is forgotten in patches too. So it still relies on reviewers catching this.

For now I request changes, as request discussion. I haven't looked closely at the patch itself, but some C headers might benefit from a synopsis. Especially due to macros that are not in a using declaration.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D147473/new/

https://reviews.llvm.org/D147473



More information about the libcxx-commits mailing list