owenpan accepted this revision.
owenpan added inline comments.

================
Comment at: clang/unittests/Format/FormatTest.cpp:20888
+               "  {  7,     5,    \"!!\" }\n"
+               "};\n",
+               Style);
----------------
galenelias wrote:
> owenpan wrote:
> > galenelias wrote:
> > > owenpan wrote:
> > > > 
> > > This is consistent with basically every single adjacent test in this 
> > > function.  While I agree that this is unnecessary, in general I error on 
> > > the side of consistency with the surrounding tests.  I'll defer to the 
> > > maintainers, just wanted to make sure this is actually the preferred 
> > > change given the numerous adjacent tests with this form.
> > If you rebase your patch, you'll see that the trailing newlines in the 
> > surrounding tests have been removed. (Even if they had not been removed, we 
> > still wouldn't want new tests to have superfluous trailing newlines.)
> Thanks for the information, looks like I missed your cleanup change by a day. 
>  In general some code-bases favor consistency over some extra gunk, so I tend 
> to default to consistency, but good to know going forward.  I've rebased now.
Np!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158795/new/

https://reviews.llvm.org/D158795

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to