MyDeveloperDay added a comment. @HazardyKnusperkek Its probably my "bad" I should said "LGTM but maybe wait for the others to comment", but I'm fundamentally ok I think with the change. (we'll just revert if it breaks stuff! ;-))
We are free to add review comments after the fact, @owenpan has been providing us with fixes and contributions to clang-format for a some time (years) so I pretty much trust them. I'm less sceptical in the review of those that contribute either on a regular basis (like yourself) or those who have done multiple features (like @owenpan). I did actually download the patch, apply it and run all the unit tests too, (because I was slightly questioning in my mind the use of "Previous" and if it could be null), but it all seemed to check out (and I tried some combinations to try and break it)) I don't think we need to wait multiple accepts (although between us I think we do this from time to time), I think the LLVM rules specify that if you have an accept then its ok, But I do feel its perfectly acceptable to review after the fact and expect the author to change it again. I was the one that added you and @curdeius as reviewers (as I normally do out of respect for your efforts), but perhaps only we know that we are the ones doing the lion share of clang-format reviewing (others might not know that). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104774/new/ https://reviews.llvm.org/D104774 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
