[PATCH] D104774: [clang-format] Fix a bug that indents else-comment-if incorrectly

MyDeveloperDay via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 24 05:07:30 PDT 2021


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



More information about the cfe-commits mailing list