You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@karaf.apache.org by Robert Varga <ni...@hq.sk> on 2018/12/01 09:02:10 UTC

Re: [PROPOSAL] Add spotless to Karaf and subprojects

On 30/11/2018 15:50, Jean-Baptiste Onofré wrote:
> Hi Lukasz,
> 
> I tried to use the "standard" style supported by spotless. It's also
> possible to use a custom eclipse formatter.

FWIW, in ODL we started off with Google formatting (if I remember
correctly, it's 5+ years now), but decided to tweak it:

- max columns = 120
- indent = 4
- imports: groups only for static/non-static, alpha-sorted
- curly braces are never standalone (to save vertical space)

The resulting (checkstyle) rules are at
https://github.com/opendaylight/odlparent/blob/master/checkstyle/src/main/resources/odl_checks.xml

Regards,
Robert


> 
> Does it what you are proposing ?
> 
> I can do that if you think it's a better way.
> 
> Thanks
> Regards
> JB
> 
> On 30/11/2018 14:26, Łukasz Dywicki wrote:
>> I'm not entirely sure if style which gets promoted is not too "wide". I
>> love approach as it reduces to bare minimum fights between IDEs and
>> their way of ordering imports and so on and makes PR review process just
>> simpler.
>>
>> Some points for formatter and verbosity of some elements:
>> 1. Two extra tabs after field doesn't make 80-100 margin:
>>     private static final EventImpl STOP_EVENT =
>>             new EventImpl(new Event("stop", Collections.emptyMap()));
>>
>>
>> 2. Extra tabs for lambdas which are just taking space:
>>             return () ->
>>                     new Iterator<String>() {
>>
>> 3. Double indention for case statements:
>>
>>             case Event.TYPE_LOGIN:
>>                 {
>>                     append(event, "username");
>>                     break;
>>                 }
>>
>>
>> Not sure how former standard java formatting rules or Sun style was
>> made, however its a minor thing.
>> Similar approach is used within Eclipse Smarthome and its fine. At
>> beginning its a pain when maven reports you - add comment here, add
>> comment there, but once you get used to it it pays off.
>>
>> Cheers,
>> Lukasz
>>
>>
>> On 23.11.2018 06:19, Jean-Baptiste Onofré wrote:
>>> Hi guys,
>>>
>>> Thoughts about spotless ?
>>>
>>> https://github.com/apache/karaf/pull/648
>>>
>>> I will update the PR today.
>>>
>>> Agree to apply spotless or we just avoid this for now ? Shall I start a
>>> formal vote ?
>>>
>>> Thanks !
>>> Regards
>>> JB
>>>
>>> On 07/11/2018 05:43, Jean-Baptiste Onofré wrote:
>>>> Hi team,
>>>>
>>>> I created a PR (https://github.com/apache/karaf/pull/648) to enable
>>>> spotless in Karaf.
>>>>
>>>> Spotless is code style checker but also formatter.
>>>>
>>>> The spotless profile I added in the PR check the style and the style can
>>>> be automatically fixed using mvn spotless:apply -Pspotless.
>>>>
>>>> I think it would be great to have this in Karaf and subprojects to have
>>>> a consistency in our code style.
>>>>
>>>> In combination with rat, it gives us a much cleaner code.
>>>>
>>>> On the other hand, I was planning to add findbugs maven plugin as well.
>>>>
>>>> Thoughts ?
>>>>
>>>> Thanks,
>>>> Regards
>>>> JB
>>>>
>>>
> 


Re: [PROPOSAL] Add spotless to Karaf and subprojects

Posted by Jean-Baptiste Onofré <jb...@nanthrax.net>.
Thanks Robert,

I like what you did. spotless suppots checkstyle formatter, so, I can 
use a modified odl_checks.xml.

I will update the PR accordingly.

Regards
JB

On 01/12/2018 10:02, Robert Varga wrote:
> On 30/11/2018 15:50, Jean-Baptiste Onofré wrote:
>> Hi Lukasz,
>>
>> I tried to use the "standard" style supported by spotless. It's also
>> possible to use a custom eclipse formatter.
> 
> FWIW, in ODL we started off with Google formatting (if I remember
> correctly, it's 5+ years now), but decided to tweak it:
> 
> - max columns = 120
> - indent = 4
> - imports: groups only for static/non-static, alpha-sorted
> - curly braces are never standalone (to save vertical space)
> 
> The resulting (checkstyle) rules are at
> https://github.com/opendaylight/odlparent/blob/master/checkstyle/src/main/resources/odl_checks.xml
> 
> Regards,
> Robert
> 
> 
>>
>> Does it what you are proposing ?
>>
>> I can do that if you think it's a better way.
>>
>> Thanks
>> Regards
>> JB
>>
>> On 30/11/2018 14:26, Łukasz Dywicki wrote:
>>> I'm not entirely sure if style which gets promoted is not too "wide". I
>>> love approach as it reduces to bare minimum fights between IDEs and
>>> their way of ordering imports and so on and makes PR review process just
>>> simpler.
>>>
>>> Some points for formatter and verbosity of some elements:
>>> 1. Two extra tabs after field doesn't make 80-100 margin:
>>>      private static final EventImpl STOP_EVENT =
>>>              new EventImpl(new Event("stop", Collections.emptyMap()));
>>>
>>>
>>> 2. Extra tabs for lambdas which are just taking space:
>>>              return () ->
>>>                      new Iterator<String>() {
>>>
>>> 3. Double indention for case statements:
>>>
>>>              case Event.TYPE_LOGIN:
>>>                  {
>>>                      append(event, "username");
>>>                      break;
>>>                  }
>>>
>>>
>>> Not sure how former standard java formatting rules or Sun style was
>>> made, however its a minor thing.
>>> Similar approach is used within Eclipse Smarthome and its fine. At
>>> beginning its a pain when maven reports you - add comment here, add
>>> comment there, but once you get used to it it pays off.
>>>
>>> Cheers,
>>> Lukasz
>>>
>>>
>>> On 23.11.2018 06:19, Jean-Baptiste Onofré wrote:
>>>> Hi guys,
>>>>
>>>> Thoughts about spotless ?
>>>>
>>>> https://github.com/apache/karaf/pull/648
>>>>
>>>> I will update the PR today.
>>>>
>>>> Agree to apply spotless or we just avoid this for now ? Shall I start a
>>>> formal vote ?
>>>>
>>>> Thanks !
>>>> Regards
>>>> JB
>>>>
>>>> On 07/11/2018 05:43, Jean-Baptiste Onofré wrote:
>>>>> Hi team,
>>>>>
>>>>> I created a PR (https://github.com/apache/karaf/pull/648) to enable
>>>>> spotless in Karaf.
>>>>>
>>>>> Spotless is code style checker but also formatter.
>>>>>
>>>>> The spotless profile I added in the PR check the style and the style can
>>>>> be automatically fixed using mvn spotless:apply -Pspotless.
>>>>>
>>>>> I think it would be great to have this in Karaf and subprojects to have
>>>>> a consistency in our code style.
>>>>>
>>>>> In combination with rat, it gives us a much cleaner code.
>>>>>
>>>>> On the other hand, I was planning to add findbugs maven plugin as well.
>>>>>
>>>>> Thoughts ?
>>>>>
>>>>> Thanks,
>>>>> Regards
>>>>> JB
>>>>>
>>>>
>>
>