curdeius added a comment.
Almost there. I hope I'm clear this time on what I'd like to see.
Thanks for the release notes update.
================
Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:1162
+
+ EXPECT_EQ(DefaultUnwrappedLines, Style.ShortNamespaceLines);
+ EXPECT_EQ("namespace ShortNamespace {\n"
----------------
Nit.
================
Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:1201-1206
+}
+
+TEST_F(ShortNamespaceLinesTest,
+ MultipleUnwrappedLine_AddsEndCommentForLongNamespace) {
+ auto Style = getLLVMStyle();
+ Style.ShortNamespaceLines = 2u;
----------------
What I meant by "merging the tests together" is just writing:
```
TEST_F(ShortNamespaceLinesTest,
MultipleUnwrappedLine) {
auto Style = getLLVMStyle();
Style.ShortNamespaceLines = 2u;
EXPECT_EQ(...);
EXPECT_EQ(...);
}
```
This will get rid of duplicated setup (even if small), and, what's more
important, show the behaviour with the same style on different cases.
Or even, but I'm not forcing you to do so, having a single test case for all
`ShortNamespaceLinesTest.*` tests. It groups the test for the same feature
together.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87587/new/
https://reviews.llvm.org/D87587
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
