ilya-biryukov added a comment. In D59887#1508991 <https://reviews.llvm.org/D59887#1508991>, @thakis wrote:
> Out of interest: The RecursiveASTVisitorTests are part of the ToolingTests > binary while this adds a new binary TokensTest. Can you say why? > > (No change needed, just curious.) The syntax library is mostly independent from the rest of tooling, so I'd rather keep everything related to it separate including the tests. I don't think we'll get anything in terms of code reuse from merging them into the same test binary either. In D59887#1508993 <https://reviews.llvm.org/D59887#1508993>, @thakis wrote: > Another comment: The new binary is called TokensTest but is in a directory > "Syntax". For consistency with all other unit test binaries, please either > rename the binary to SyntaxTests, or rename the directory to "Tokens". (From > the patch description, the former seems more appropriate.) Note the missing > trailing "s" in the binary name too. Agree with renaming the binary. In fact, the not-yet-landed revisions also use `SyntaxTests`, and I should've named it this way from the start. I'll land a patch. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59887/new/ https://reviews.llvm.org/D59887 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
