HazardyKnusperkeks added a comment. In D104774#2838287 <https://reviews.llvm.org/D104774#2838287>, @MyDeveloperDay wrote:
> @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). No problem here. :) I think the change is small enough and the fix obvious. It was just a small comment. 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
