Re: [ovirt-devel] Why are Java checkstyle checks applied to test code?

----- Original Message ----- In general, you are right, but I feel this depends on how (what for) we use Checkstyle in oVirt Engine project. There are different kinds of code validation: a, general "avoid bad coding patterns" validation (i.e. code lint), for this we use FindBugs b, project-specific *general* code style validation (i.e. NewlineAtEndOfFile), for this we use Checkstyle c, project-specific *business* code style validation (i.e. NlsCheck), for this we also use Checkstyle While it makes sense to apply (b) to "test" code, IMHO it makes no sense to apply (c) to "test" code. For example, checks like NlsCheck (//$NON-NLS-X) force us to pollute "test" code as it makes no sense to localize strings in "test" code anyway. Therefore, we should split (b) and (c) into two separate executions: - apply (b) for "src" and "test" code - apply (c) only for "src" code Each execution would map to separate maven-checkstyle-plugin:check goal bound to Maven "compile" phase. The original issue was that Checkstyle's parser failed to parse Java file if this file contains { and } characters in string, for example: private static final String PLACE_HOLDER_STRING = "\\{(\\d+)\\}"; This is internal Checkstyle issue, not an issue with our code. Because parsing happens *before* applying any Checkstyle checks, suppressing the problematic portions of Java file will not solve the problem. The only way to solve this problem is to exclude such files from Checkstyle. So I have a question, what to do in such cases, i.e. when Checkstyle fails on some internal (failed to parse, etc.) error, while our code is perfectly valid? (I don't see "adapting our code to make it parse- able by Checkstyle" as a proper solution.) As I wrote above, the issue is with Checkstyle, not with our code. If it happened for "src" code, I'd had to exclude that file from being analyzed (parsed & processed) by Checkstyle, anyway. That will not work, see my comments above. +1, I think we should review existing Checkstyle config.