[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