[PATCH] D117939: [clang-tidy] Add more documentation about check development

[Richard via Phabricator via cfe-commits]

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