cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tutkowski, Mike" <Mike.Tutkow...@netapp.com>
Subject Re: Checkstyle / code style / reformatting
Date Fri, 29 Sep 2017 17:15:35 GMT
That sounds like another one to wait for until code freeze. :)

On Sep 29, 2017, at 7:45 AM, Daan Hoogland <daan.hoogland@gmail.com<mailto:daan.hoogland@gmail.com>>
wrote:

ohw dear Mike, I am changing the log4j version, brrrrrr.

On Fri, Sep 29, 2017 at 3:11 PM, Tutkowski, Mike <Mike.Tutkowski@netapp.com<mailto:Mike.Tutkowski@netapp.com>>
wrote:

I wonder if it wouldn’t be best to wait on this until master is in code
freeze. At that point, it seems like we could add whatever rules we’d like
and have almost no impact on pending master PRs.

For me personally, although I haven’t opened it yet, I have a large PR I
plan to open in the coming weeks and I’m not looking forward to potentially
changing a million formatting issues in it due to new formatting rules. :)

On 9/29/17, 1:44 AM, "Marc-Aurèle Brothier" <marco@exoscale.ch<mailto:marco@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
Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message