Don't abort build when there are checkstyle violations #3006
Conversation
LinusDietz
added
build-system
type: code-quality
|
Why? This improves our code quality. If not the author, who else will clean up the code? Which checkstyle rules are annoying? We all know (do we?) that JabRef's code was (and still is) a mess (at some places). I strongly believe that checkstyle rules are one solution to the problem. |
|
I give it a go. If the active developers don't see any value in checkstyle checks, I give it a go. Refs #2996 |
|
Well, also I think checkstyle warnings are annoying, they necessary. And it's not much work when you just have to press ctrl+f in eclipse to do it. |
|
Hey @koppor, the intention is not to abolish checkstyle, but to improve the coding experience. When working on a PR, the most important thing are the unit tests to pass. When executing gradle check it takes 59 secs to only compile the java code and run the checkstyle tests. If there is ANY problem because I didn't hit the code formatter the build will abort at this point and the tests are not executed. (see also #2996 (comment)) That's why suggested to make them optional. Of course they should be adhered before merged into master. In an ideal case they SHOULD fail on travis, but not locally. Does anybody know if that's possible in an elegant way? |
|
If I recall the discussion correctly, then we agreed on the following: The build should execute even if their are checkstyle warnings (so that you get feedback on the tests), but it should still fail. Is this what this PR achieves? I hate checkstyle and complain all the time, but I concede that the rules improve code quality. |
* upstream/master: (30 commits) Add preference migration for keybdingings (#3007) Shutdown previus AutosaveManager and BackupManager during SaveAs (#2994) Run Checkstyle task after Test task (#3010) Mark LibraryOfCongressTest as a fetcher test (#3012) When browsing through the MainTable remember which EntryEditor tab was open (#3011) Improve performance of journal abbreviation loader (#3009) Update checkstyle 7.6.1 -> 8.0 Don't abort build when there are checkstyle violations (#3006) Reimplement content selectors (#3003) Only do a back up for bigger changes or if a different field is edited (#3004) Listen to change events for setting dirty status of database (#3001) Fix #2967: MathSciNet tab works again Fix #2902: Tab in entry editor moves to next text area Fix #2946: external changes are now correctly shown in the entry editor Fix #2998: improve auto completion (#3002) Fix mac keybinding by replacing ctrl it with meta key (#3000) Less backups (#2995) [WIP] Complete rework of the auto completion (#2965) Eclipse J Add switch indentation for Eclipse and add some new missing formatting options ...
* upstream/master: Add preference migration for keybdingings (#3007) Shutdown previus AutosaveManager and BackupManager during SaveAs (#2994) Run Checkstyle task after Test task (#3010) Mark LibraryOfCongressTest as a fetcher test (#3012) When browsing through the MainTable remember which EntryEditor tab was open (#3011) Improve performance of journal abbreviation loader (#3009) Update checkstyle 7.6.1 -> 8.0 Don't abort build when there are checkstyle violations (#3006) Reimplement content selectors (#3003) Only do a back up for bigger changes or if a different field is edited (#3004) Listen to change events for setting dirty status of database (#3001) Fix #2967: MathSciNet tab works again Fix #2902: Tab in entry editor moves to next text area Fix #2946: external changes are now correctly shown in the entry editor Fix #2998: improve auto completion (#3002) Fix mac keybinding by replacing ctrl it with meta key (#3000) Less backups (#2995) [WIP] Complete rework of the auto completion (#2965)
Upon wish of @lenhard, nothing more to say here