riccibruno added a comment. In D57104#1370025 <https://reviews.llvm.org/D57104#1370025>, @aaron.ballman wrote:
> In D57104#1369985 <https://reviews.llvm.org/D57104#1369985>, @steveire wrote: > > > There's definitely a better possible ordering in two commits: > > > > 1. Introduce `::Create` and port to it > > 2. Use trailing objects, taking advantage of the fact that `::Create` > > exists. > > > > That would make it clear in the future to other people because both > > commits would be cleaner, both commit messages would say what the commit > > does, and neither commit would have the noise of the other change. > > > > Not splitting this commit makes it less reviewable to people who are not > > around today. > > > I would find reviewing that more confusing because the two reviews are > inextricably linked. There's no need for a `Create()` method without the > other changes, and the other changes require a `Create()` method. I get what > you mean about keeping reviews concise, but at some point you can split the > reviews so much that it becomes really difficult to perform a sensible review > because you get too much information in isolation. +1 You can cut a commit into arbitrarily small pieces. As a silly example I could inherit from `llvm::TrailingObjects` in one commit. Then remove the `Stmt **` in another one and so on. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57104/new/ https://reviews.llvm.org/D57104 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
