curdeius requested changes to this revision.
curdeius added a comment.
This revision now requires changes to proceed.
Please regroup tests and add release notes.
================
Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:1126
+
+ EXPECT_EQ(UnwrappedLines, Style.ShortNamespaceLines);
+}
----------------
This could be tested together with another test.
================
Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:1129
+
+TEST_F(ShortNamespaceLinesTest,
+ ZeroUnwrappedLines_DoesNotAddEndCommentForOneLinerNamespace) {
----------------
In general, I think you can merge some test cases to match the current testing
"style".
================
Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:1141
+ auto Style = getLLVMStyle();
+ Style.ShortNamespaceLines = 0;
+
----------------
From the tests one could imply that 0 means "never add comments", or "add
comments for non empty namespaces". Please add a test with at least one line in
the namespace.
================
Comment at: clang/unittests/Format/NamespaceEndCommentsFixerTest.cpp:1161
+TEST_F(ShortNamespaceLinesTest,
+ OneUnwrappedLine_AddsEndCommentForLongNamespace) {
+ EXPECT_EQ("namespace LongNamespace {\n"
----------------
You should check that Style.ShortNamespaceLines is 1 here.
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
