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

Björn Schäpers via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jun 24 12:28:31 PDT 2021


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



More information about the cfe-commits mailing list