User Details
- User Since
- Jul 29 2013, 1:59 AM (466 w, 2 d)
Today
Address comments.
Mon, Jul 4
LGTM. Thanks for addressing my comments.
Sun, Jul 3
Sun, Jun 26
Fri, Jun 24
LGTM with Owen's suggestion.
Tue, Jun 21
Thu, Jun 16
Wed, Jun 15
Tue, Jun 14
Does this patch really fix https://github.com/llvm/llvm-project/issues/54703?
If so, please add test for it. Otherwise remove the link from the summary (and if possible handle it in another review).
Mon, Jun 13
Fri, Jun 10
It seems like a breaking change that may be painful for users of GNU style. @MyDeveloperDay, wdyt?
Thu, Jun 9
Good finding!
Wed, Jun 8
LGTM.
Still looks good. Was there a particular case where the previous version didn't work?
@Eitot, do you need help landing this?
Tue, Jun 7
LGTM. Thanks!
Apart from some missing tests, looks promising!
Jun 5 2022
Great!
Jun 1 2022
Ok, I'm not blocking this patch. I'll take a look to see whether we can add some tests.
May 26 2022
May 25 2022
LGTM. Good catch for this bug!
LGTM. Thanks a lot!
Do you have commit rights or need some help landing this?
LGTM.
Would it be possible to add a test please?
May 23 2022
Could you please add test cases with non-empty enums both with and without comments please like in the bug report?
Closing as it landed in https://github.com/llvm/llvm-project/commit/130a9cc0a5e25e3be8ff3738518e86ae3ae0b5ba.
May 22 2022
Ok. So we mainly missed braces on complex conditionals. LGTM.
LGTM. It seems it has landed already.
May 21 2022
May 20 2022
If you can think of other cases that may misbehave, I'll be happy to test (and fix) these.
Thanks! LGTM.
Thanks for the patch!
Could you please add a unit test in unittest/Format/FormatTest.cpp instead of in lit-based test/?
Is there a bug report that your patch fixes?
LGTM
May 17 2022
Reverted for now.
May 16 2022
Sure, I'll have a look.
It seems that even this:
MACRO_BEGIN #if A int f(); #else int f(); #endif
gets misindented:
MACRO_BEGIN #if A int f(); #else int f(); #endif
May 15 2022
LGTM. I like it! Thanks!
May 13 2022
LGTM. Good finding!
May 12 2022
LGTM. Tell us if you need help landing this.
Thanks a lot for your contribution!
May 11 2022
LGTM with nits.
Do you need help landing this?
May 10 2022
Thanks for creating the bug report. A few more comments.
May 9 2022
Address comment: simplify.
May 7 2022
FYI, I like to have a bug report because people that encounter the same problem (in an older release) can find it easily, and see if/when it was fixed. It also avoid working on the same thing by different people.
Thanks for working on this!
Is there a bug report somewhere? If not, may you create one please?
May 6 2022
Simplify. Address comments.
May 5 2022
LGTM
May 4 2022
Just one last thought, shouldn't we have a test case close to what was reported in the issue? I.e. with PP directives.
LGTM!
May 3 2022
Looks ok, but please let someone else have a look too.
May 2 2022
LGTM.
Apr 28 2022
Feel free to revert and add a regression test please. I'll take a look.