[GitHub] [maven-checkstyle-plugin] bmhm commented on issue #17: [MCHECKSTYLE-385] rework code to use a Violation.java class.

[GitBox <gi...@apache.org>]

bmhm commented on issue #17: [MCHECKSTYLE-385] rework code to use a Violation.java class.
URL: https://github.com/apache/maven-checkstyle-plugin/pull/17#issuecomment-564945721
 
 
   Things to think of:
   
    - Some javadoc. Especially the difference between `source` and `file` does not seem clear.
    - some better variable naming for the same variables. I used the names which were already present and undocumented in the code.
    - This code uses Java 8 streams, but I did not change any existing code where not necessary. For example, I kept the `for` loop.
    - The code style (brace on new line) looks wired in lambdas. Also, lambdas really should not go beyong ~5 lines or so. That should also go in the checkstyle config for this project.
    - I cannot switch `//CHECKSTYLE:OFF` for the constructor of the `Validation` class, which is a bummer. This is the reason why the class will never be a real immutable class.
    - The ignore-check is now done twice. But fixing this is an easy improvement.
    - On large projects, this commit might lead to a bit more heap usage, as all objects are collected first. This may be unwanted behaviour.
    - The "fail on first" behaviour was not changed, because it was never queried in the first place for the `check` goal.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services