LegalizeAdulthood marked an inline comment as done. LegalizeAdulthood added inline comments.
================ Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:362 +Private custom matchers are a good example of auxiliary support code for your check +that is best tested with a unit test. It will be easier to test your matchers or +other support classes by writing a unit test than by writing a ``FileCheck`` integration ---------------- If I change the wording from "is best tested with a unit test" to "can be tested with a unit test", would that alleviate the concern? I want to encourage appropriate testing and unit testing complex helper code is better than integration testing helper code. I find it easier to have confidence in private matchers if they are unit tested and I've recently had a few situations where I had to write relatively complex helper functions to analyze raw text that I felt would have been better tested with a unit test than an integration test. ================ Comment at: clang-tools-extra/docs/clang-tidy/Contributing.rst:389-390 +- Test your check under both Windows and Linux environments. +- Watch out for high false positive rates; ideally there should be none, but a + small number may be tolerable. High false positive rates prevent adoption. +- Consider configurable options for your check to deal with false positives. ---------------- ymandel wrote: > Is it worth giving a rule-of-thumb? Personally I'd go with < 10%, all things > being equal. A check for a serious bug may reasonably have a higher false > positive rate, and trivial checks might not justify *any* false positives. > But, if neither of these apply, I'd recommend 10% as the default. I'm OK with rule-of-thumb 10% advice. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117939/new/ https://reviews.llvm.org/D117939 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
