riccibruno added a comment. In D57104#1368292 <https://reviews.llvm.org/D57104#1368292>, @steveire wrote:
> In D57104#1368072 <https://reviews.llvm.org/D57104#1368072>, @riccibruno > wrote: > > > In D57104#1368055 <https://reviews.llvm.org/D57104#1368055>, @steveire > > wrote: > > > > > Splitting the introduction of and porting to `Create` would significantly > > > reduce the number of files touched by the 'real' change in this commit, > > > and therefore reduce noise in the commit (following the idea of "do one > > > thing per commit" to make the code reviewable in the future). > > > > > > However, if you're opposed to that, it's not a hard requirement. > > > > > > To be honest I don't really see the point. > > > Yes, the `Create` refactor a simple change. The point is to remove the noise > of the simple change from the complex change so that the complex change > becomes visible. > > It would help reviewers like myself who are less familiar with what > refactoring for tail allocation involves. I couldn't read your commit and > know how to do it for another class because this commit is full of noise. > > Anyway, if making the code reviewable (also in the future!) like that is not > important enough, I won't block this one! :) You are right that readability and review-ability is pretty important. My apologies for making this an unreadable mess :) Let me clean this patch a little bit (along with addressing Aaron's comments). 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
