The checkstyle rules can be imported in pretty much any IDE to help during coding and to display the errors immediately. To speed up the detection of checkstyle errors, we could edit the post commit hooks to run a mvn command only for the checkstyle rules, and then do the usual stuff.
I'm not in favor to alter the code after being approved. The PRs should be "correct" on all points: code style, functionality, tests, doc On Fri, Sep 29, 2017 at 10:19 AM, Daan Hoogland <daan.hoogl...@gmail.com> wrote: > I am a fan of convention and think everyone should have some. Strict > enforcing of non-functionals, I'm not to big on. I see what you want to > achieve here but am reluctant not to -1 any strictness. If we do it with a > post-commit-hook (or if such a thing exists a post-merge-hook) we will > allow for code to be altered after being approved, won't we? > > On Fri, Sep 29, 2017 at 9:44 AM, Marc-Aurèle Brothier <ma...@exoscale.ch> > wrote: > > > Hi everyone, > > > > Would you think it's worth tightening the checkstyle rules and run a > > reformatting pass on the code base to align everything slowly? I know it > > will hide changes using the blame functionality, and would force people > to > > edit some PR. Is the trade-off worth it for you? > > > > The idea would be to add one by one those extra checkstyle rules, and do > > the code change/reformat accordingly each time with one rule in one PR. A > > new rule should first be accepted by the community before starting on the > > PR to do the changes (otherwise it might be a lot of wasted time). When > > merged, a new rule can be added. > > > > The code style that bothers me most right now for example is blocks > without > > braces, so my first proposal would be: > > > > <module name="NeedBraces"> > > <property name="allowSingleLineStatement" value="true"/> > > </module> > > > > I have a lot more to add, but as said, one at a time ;-) > > > > The list of rules: http://checkstyle.sourceforge.net/checks.html > > The current rules: > > https://github.com/apache/cloudstack/blob/master/tools/ > > checkstyle/src/main/resources/cloud-style.xml > > > > What do you think? > > > > Marc-Aurèle > > > > > > -- > Daan >
