Re: [PROPOSAL] Add spotless to Karaf and subprojects
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() {
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
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() {
>>
>> 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
>>>
>
signature.asc
Description: OpenPGP digital signature
