Issue 4248 - Add codestyle check

Issue 4248 - Add codestyle check

Changes for 'codingtools' repository

[anton, 2017-10-06 11:29:53]

https://codereview.adblockplus.org/29567648/diff/29567649/checkstyle/java/res...
File checkstyle/java/res/header.txt (right):

https://codereview.adblockplus.org/29567648/diff/29567649/checkstyle/java/res...
checkstyle/java/res/header.txt:3: * Copyright (C) 2006-present eyeo GmbH
Note i've updated end year comparable to patch set #6 from
https://codereview.adblockplus.org/29347926/

[jens, 2017-10-11 12:18:27]

On 2017/10/06 11:29:53, anton wrote:
>
https://codereview.adblockplus.org/29567648/diff/29567649/checkstyle/java/res...
> File checkstyle/java/res/header.txt (right):
> 
>
https://codereview.adblockplus.org/29567648/diff/29567649/checkstyle/java/res...
> checkstyle/java/res/header.txt:3: * Copyright (C) 2006-present eyeo GmbH
> Note i've updated end year comparable to patch set #6 from
> https://codereview.adblockplus.org/29347926/

Besides the header text LGTM

[jens, 2017-10-11 12:18:34]

https://codereview.adblockplus.org/29567648/diff/29567649/checkstyle/java/res...
File checkstyle/java/res/header.txt (right):

https://codereview.adblockplus.org/29567648/diff/29567649/checkstyle/java/res...
checkstyle/java/res/header.txt:11: * MERCHANTABILITY or FITNESS FOR A PARTICULAR
PURPOSE.  See the
Maybe we should fix the double whitespace here and in the paragraph below?

[anton, 2017-10-11 12:22:27]

https://codereview.adblockplus.org/29567648/diff/29567649/checkstyle/java/res...
File checkstyle/java/res/header.txt (right):

https://codereview.adblockplus.org/29567648/diff/29567649/checkstyle/java/res...
checkstyle/java/res/header.txt:11: * MERCHANTABILITY or FITNESS FOR A PARTICULAR
PURPOSE.  See the
On 2017/10/11 12:18:33, jens wrote:
> Maybe we should fix the double whitespace here and in the paragraph below?

It seems to be reasonable but in adblockpluscore double space is used:
https://github.com/adblockplus/adblockpluscore/blob/master/chrome/content/ui/...

So i'd leave it as-is or consult with someone with it.

[anton, 2017-10-11 12:23:51]

On 2017/10/11 12:22:27, anton wrote:
>
https://codereview.adblockplus.org/29567648/diff/29567649/checkstyle/java/res...
> File checkstyle/java/res/header.txt (right):
> 
>
https://codereview.adblockplus.org/29567648/diff/29567649/checkstyle/java/res...
> checkstyle/java/res/header.txt:11: * MERCHANTABILITY or FITNESS FOR A
PARTICULAR
> PURPOSE.  See the
> On 2017/10/11 12:18:33, jens wrote:
> > Maybe we should fix the double whitespace here and in the paragraph below?
> 
> It seems to be reasonable but in adblockpluscore double space is used:
>
https://github.com/adblockplus/adblockpluscore/blob/master/chrome/content/ui/...
> 
> So i'd leave it as-is or consult with someone with it.

Added Vasiliy as 'codingtools' repository owner. @Vasiliy, what do you think?

[jens, 2017-10-11 12:24:34]

On 2017/10/11 12:22:27, anton wrote:
>
https://codereview.adblockplus.org/29567648/diff/29567649/checkstyle/java/res...
> File checkstyle/java/res/header.txt (right):
> 
>
https://codereview.adblockplus.org/29567648/diff/29567649/checkstyle/java/res...
> checkstyle/java/res/header.txt:11: * MERCHANTABILITY or FITNESS FOR A
PARTICULAR
> PURPOSE.  See the
> On 2017/10/11 12:18:33, jens wrote:
> > Maybe we should fix the double whitespace here and in the paragraph below?
> 
> It seems to be reasonable but in adblockpluscore double space is used:
>
https://github.com/adblockplus/adblockpluscore/blob/master/chrome/content/ui/...
> 
> So i'd leave it as-is or consult with someone with it.

Yeah, I think it's like this in all projects. But I don't see a reason for
keeping it like that in the long run.

[Vasily Kuznetsov, 2017-10-11 15:32:43]

On 2017/10/11 12:24:34, jens wrote:
> On 2017/10/11 12:22:27, anton wrote:
> >
>
https://codereview.adblockplus.org/29567648/diff/29567649/checkstyle/java/res...
> > File checkstyle/java/res/header.txt (right):
> > 
> >
>
https://codereview.adblockplus.org/29567648/diff/29567649/checkstyle/java/res...
> > checkstyle/java/res/header.txt:11: * MERCHANTABILITY or FITNESS FOR A
> PARTICULAR
> > PURPOSE.  See the
> > On 2017/10/11 12:18:33, jens wrote:
> > > Maybe we should fix the double whitespace here and in the paragraph below?
> > 
> > It seems to be reasonable but in adblockpluscore double space is used:
> >
>
https://github.com/adblockplus/adblockpluscore/blob/master/chrome/content/ui/...
> > 
> > So i'd leave it as-is or consult with someone with it.
> 
> Yeah, I think it's like this in all projects. But I don't see a reason for
> keeping it like that in the long run.

The recommended license header text from GNU uses double spaces after periods
[1]. I think we should just follow this recommendation. It's true, that using
two spaces to separate sentences is a somewhat controversial practice (see [2]),
but in this case I'm quite happy to let GNU themselves worry about typographic
nitpicking.

[1]: https://www.gnu.org/licenses/gpl-howto.en.html
[2]: https://en.wikipedia.org/wiki/Sentence_spacing

[jens, 2017-10-12 07:53:33]

On 2017/10/11 15:32:43, Vasily Kuznetsov wrote:
> On 2017/10/11 12:24:34, jens wrote:
> > On 2017/10/11 12:22:27, anton wrote:
> > >
> >
>
https://codereview.adblockplus.org/29567648/diff/29567649/checkstyle/java/res...
> > > File checkstyle/java/res/header.txt (right):
> > > 
> > >
> >
>
https://codereview.adblockplus.org/29567648/diff/29567649/checkstyle/java/res...
> > > checkstyle/java/res/header.txt:11: * MERCHANTABILITY or FITNESS FOR A
> > PARTICULAR
> > > PURPOSE.  See the
> > > On 2017/10/11 12:18:33, jens wrote:
> > > > Maybe we should fix the double whitespace here and in the paragraph
below?
> > > 
> > > It seems to be reasonable but in adblockpluscore double space is used:
> > >
> >
>
https://github.com/adblockplus/adblockpluscore/blob/master/chrome/content/ui/...
> > > 
> > > So i'd leave it as-is or consult with someone with it.
> > 
> > Yeah, I think it's like this in all projects. But I don't see a reason for
> > keeping it like that in the long run.
> 
> The recommended license header text from GNU uses double spaces after periods
> [1]. I think we should just follow this recommendation. It's true, that using
> two spaces to separate sentences is a somewhat controversial practice (see
[2]),
> but in this case I'm quite happy to let GNU themselves worry about typographic
> nitpicking.
> 
> [1]: https://www.gnu.org/licenses/gpl-howto.en.html
> [2]: https://en.wikipedia.org/wiki/Sentence_spacing

Thanks for clarification, Vasily.
In this case, LGTM

[diegocarloslima, 2017-10-16 12:31:15]

https://codereview.adblockplus.org/29567648/diff/29567649/checkstyle/java/rul...
File checkstyle/java/rules/eyeo_production_checks.xml (right):

https://codereview.adblockplus.org/29567648/diff/29567649/checkstyle/java/rul...
checkstyle/java/rules/eyeo_production_checks.xml:98: <!-- See
http://checkstyle.sf.net/config_imports.html -->
I know that the comments here are based on sun_checks.xml, but I just think that
would be better to just close the comments without adding extra spaces to align
them, since they can eventually get misaligned when the comments changes, like
here.

[diegocarloslima, 2017-12-13 11:17:19]

On 2017/10/16 12:31:15, diegocarloslima wrote:
>
https://codereview.adblockplus.org/29567648/diff/29567649/checkstyle/java/rul...
> File checkstyle/java/rules/eyeo_production_checks.xml (right):
> 
>
https://codereview.adblockplus.org/29567648/diff/29567649/checkstyle/java/rul...
> checkstyle/java/rules/eyeo_production_checks.xml:98: <!-- See
> http://checkstyle.sf.net/config_imports.html -->
> I know that the comments here are based on sun_checks.xml, but I just think
that
> would be better to just close the comments without adding extra spaces to
align
> them, since they can eventually get misaligned when the comments changes, like
> here.

LGTM, with a minor note regarding comments