Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id A81CB200C17 for ; Fri, 10 Feb 2017 17:08:00 +0100 (CET) Received: by cust-asf.ponee.io (Postfix) id A52EF160B5C; Fri, 10 Feb 2017 16:08:00 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 7EB9C160B5B for ; Fri, 10 Feb 2017 17:07:59 +0100 (CET) Received: (qmail 31700 invoked by uid 500); 10 Feb 2017 16:07:58 -0000 Mailing-List: contact log4j-dev-help@logging.apache.org; run by ezmlm Precedence: bulk List-Unsubscribe: List-Help: List-Post: List-Id: "Log4J Developers List" Reply-To: "Log4J Developers List" Delivered-To: mailing list log4j-dev@logging.apache.org Received: (qmail 31690 invoked by uid 99); 10 Feb 2017 16:07:58 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd1-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 10 Feb 2017 16:07:58 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd1-us-west.apache.org (ASF Mail Server at spamd1-us-west.apache.org) with ESMTP id 40C1CC3AEB for ; Fri, 10 Feb 2017 16:07:58 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 2.379 X-Spam-Level: ** X-Spam-Status: No, score=2.379 tagged_above=-999 required=6.31 tests=[DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, HTML_MESSAGE=2, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, RCVD_IN_SORBS_SPAM=0.5, SPF_PASS=-0.001] autolearn=disabled Authentication-Results: spamd1-us-west.apache.org (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com Received: from mx1-lw-us.apache.org ([10.40.0.8]) by localhost (spamd1-us-west.apache.org [10.40.0.7]) (amavisd-new, port 10024) with ESMTP id iiJI2ZPc1sX9 for ; Fri, 10 Feb 2017 16:07:57 +0000 (UTC) Received: from mail-lf0-f51.google.com (mail-lf0-f51.google.com [209.85.215.51]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with ESMTPS id 794765F342 for ; Fri, 10 Feb 2017 16:07:56 +0000 (UTC) Received: by mail-lf0-f51.google.com with SMTP id n124so23983132lfd.2 for ; Fri, 10 Feb 2017 08:07:56 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to; bh=BRcwCuSLMHgooNGvYELkNnfMxw2su89EspNl/yh/y+M=; b=XeYYxga068aEZcj858s70we/h7QmtNjxyzLxUI94TNslvju7+U6wq+LfZYt2TbLidQ BE/PXUEsNDlJmfqjXrm/huUqhaELkJc2WQoer76Brp0twH1g2yQZCbzfqgyasNb3fCHh /jO7XRjwtQfcQ7XXzTJD8lC5ISFcQJvcAud6vVMjkFAC88WEkeSziD4yZ7SKVFCX8cHg 40jmX7tA7/0ZhwCzhPAG5sM+257yqar3v4T9g1kVBtu0vCRq0JklPmJhjCQFIAsS1yZ8 SZ5D4WwIPviyFkcej8e+46YMswINTfq2vYBFlMVQ03ujxE2OmY1IftYEyLSlAV1pLzQJ mPPA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=BRcwCuSLMHgooNGvYELkNnfMxw2su89EspNl/yh/y+M=; b=et+mjN+l8BHIEY2Bh0xUrqtv9H9FEaYQhEveHZwn+TREGc/wWBNZzXcCHoNel7vZc2 1p4iB70rxalbk9CtVAkmv7fne/loPRLRZ5k3TlJacYgedeyEb++zZWXNYNZU9feowmMr 86lTpyl2wItu8u77BHvhRClninUq0bzE3d/14o4TvpY9LQLMVEyitAO0Uj31ZGHr6zW6 0yPzSuDXvjvhBZQB9XTGsngM8WI+wfsRF0LfvhRJESHQG98kb9RgH8ImsnAuAU6G0LCP ifORLDQPLZ4EOWwhuV9+uun7TJcN/Y9S+VJMcuICZIANI2e399mjjHZvmAhRc+/9BmCX L2EQ== X-Gm-Message-State: AMke39ld/HMyT9EZwQwPftlGKraJHw/Tc3hJ9jl5rcnfXzLg0db/cvGZkbjgQzfvow94MFsuMrWjS4Kdo+Oo7Q== X-Received: by 10.46.88.3 with SMTP id m3mr3385833ljb.24.1486742871226; Fri, 10 Feb 2017 08:07:51 -0800 (PST) MIME-Version: 1.0 Received: by 10.25.158.76 with HTTP; Fri, 10 Feb 2017 08:07:50 -0800 (PST) In-Reply-To: References: <19A44C71-5241-48D4-936E-8B04BB22BFCA@dslextreme.com> From: Matt Sicker Date: Fri, 10 Feb 2017 10:07:50 -0600 Message-ID: Subject: Re: Regarding Checkstyle, PMD, and formatting To: Log4J Developers List Content-Type: multipart/alternative; boundary=f4030438f198b6432605482f4caa archived-at: Fri, 10 Feb 2017 16:08:00 -0000 --f4030438f198b6432605482f4caa Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable The "unnecessary parenthesis" rule is somewhat annoying. While it has good intentions, it'll also flag something like this: foo || (bar && baz) Sure, && has higher precedence than || (had to look it up just now), but who can remember those kinds of rules anyways? On 10 February 2017 at 09:36, Remko Popma wrote: > +1 on braces > > On Sat, Feb 11, 2017 at 12:35 AM, Apache > wrote: > >> You don=E2=80=99t really have to use final everywhere. If you don=E2=80= =99t, Gary will >> fix it ;-) >> >> Actually, I really do prefer most of our check style rules, but not >> enough to yell and scream about it. The one that bothers me the most is >> that I want braces wherever they are optional. >> >> Ralph >> >> On Feb 10, 2017, at 8:09 AM, Matt Sicker wrote: >> >> At work, I've switched from final everywhere to final everywhere but >> local variables while maintaining effective finality instead. I just wis= h >> Java had final be the default. >> >> On 10 February 2017 at 05:34, Remko Popma wrote: >> >>> Generally agree except that we agreed that the final qualifier was >>> optional. This may not be easy (or possible?) to verify automatically >>> anyway. >>> >>> Otherwise all looks reasonable. >>> >>> Sent from my iPhone >>> >>> On Feb 10, 2017, at 17:55, Mikael St=C3=A5ldal >>> wrote: >>> >>> Seems reasonable. >>> >>> On Fri, Feb 10, 2017 at 5:56 AM, Gary Gregory >>> wrote: >>> >>>> I agree with all that! :-) >>>> >>>> Gary >>>> >>>> >>>> On Feb 9, 2017 7:05 PM, "Matt Sicker" wrote: >>>> >>>> I was browsing through the site and took a look at the component >>>> reports. Checkstyle alone seems close to pointless as there are over 2= 00 >>>> errors in log4j-api alone. log4j-core has over 2000 errors. Even new f= iles >>>> that were formatted with our formatter settings such as the >>>> CassandraAppender plugin have import ordering errors. I also disagree = with >>>> some of the rules configured, but that doesn't really matter when we d= on't >>>> enforce it in the first place. >>>> >>>> Anyways, what's the point of configuring this and adding checkstyle >>>> comments yet not even using it? The only project I've come across in t= he >>>> wild so far that has checkstyle configured properly was Spring Boot, a= nd >>>> your pull request has to pass the checkstyle check to even be mergeabl= e. >>>> >>>> Perhaps if we wish to actually use it, we could loosen the rules down >>>> to a much smaller set that actually matches the formatter settings in >>>> src/ide/. If the rules matched our code base, then we could also have >>>> Jenkins run checkstyle checks which would keep us informed when we mes= s up, >>>> and it would also be useful for pull requests (I've had to reformat ma= ny >>>> patches in the past). >>>> >>>> Related, there's the style guide >>> g4j/2.x/javastyle.html> which I'm pretty sure I've never even looked >>>> at before. This could also be normalized with our formatter files. I'v= e >>>> generally thought of our code style summarized as: >>>> >>>> * 4 space indent >>>> * use final >>>> * no star imports outside tests (and those should generally be static >>>> imports) >>>> * imports should be in some sort of alphabetical order (this is really >>>> difficult to match between IntelliJ and Eclipse for some reason; I've = had >>>> rather obnoxious fights about this in the past thanks to >>>> import-order-induced merge conflicts) >>>> * try to stick to unix line endings, but we're rather mixed still >>>> * every file needs a license header unless it's impossible to include >>>> comments >>>> * use CamelCaseClassNames, even for acronyms >>>> * no hungarian notation or other distracting naming conventions >>>> * otherwise stick to typical Sun style that everyone basically >>>> recognizes (that the JDK doesn't even use itself) >>>> >>>> -- >>>> Matt Sicker >>>> >>>> >>>> >>> >>> >>> -- >>> [image: MagineTV] >>> >>> *Mikael St=C3=A5ldal* >>> Senior software developer >>> >>> *Magine TV* >>> mikael.staldal@magine.com >>> Grev Turegatan 3 | 114 46 Stockholm, Sweden | www.magine.com >>> >>> >>> Privileged and/or Confidential Information may be contained in this >>> message. If you are not the addressee indicated in this message >>> (or responsible for delivery of the message to such a person), you may >>> not copy or deliver this message to anyone. In such case, >>> you should destroy this message and kindly notify the sender by reply >>> email. >>> >>> >> >> >> -- >> Matt Sicker >> >> >> > --=20 Matt Sicker --f4030438f198b6432605482f4caa Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
The "unnecessary parenthesis" rule is somewhat a= nnoying. While it has good intentions, it'll also flag something like t= his:

foo || (bar && baz)

Sure, && has higher precedence than || (had to look it up just no= w), but who can remember those kinds of rules anyways?

On 10 February 2017 at 09:= 36, Remko Popma <remko.popma@gmail.com> wrote:
+1 on braces=C2=A0

On Sat, Feb 11, 2017 at 12:35 AM, Apache &l= t;ralph.goe= rs@dslextreme.com> wrote:
<= div style=3D"word-wrap:break-word">
You don=E2=80=99t really have to us= e final everywhere. If you don=E2=80=99t, Gary will fix it ;-)
Actually, I really do prefer most of our check style rules, bu= t not enough to yell and scream about it. The one that bothers me the most = is that I want braces wherever they are optional.

= Ralph

On Feb 10, 2017, at 8:09 AM, Matt Sicker <boards@gmail.com> wro= te:

At work, I've switched from final ev= erywhere to final everywhere but local variables while maintaining effectiv= e finality instead. I just wish Java had final be the default.

On 10 February 2017 at 05:34,= Remko Popma=C2=A0<remko.popma@gmail.com><= span class=3D"m_-3191456273204611330m_-7970033867695405500Apple-converted-s= pace">=C2=A0wrote:
Generally agree except that we agreed that the final qualifier was optiona= l.=C2=A0 This may not be easy (or possible?) to verify automatically anyway= .=C2=A0

Otherwise = all looks reasonable.=C2=A0

Sent from my iPhone

On Feb 10, 201= 7, at 17:55, Mikael St=C3=A5ldal <mikael.staldal@magine.com> wrote:

Seems reasonable.
=

On Fri, Feb 10, 2= 017 at 5:56 AM, Gary Gregory=C2=A0<garydgregory@gmail.c= om>=C2=A0wrote:
I agree with all that! :-)

Gary
=


= On Feb 9, 2017 7:05 PM, "Matt Sicker" <boards@gmail.com> wrote:
I was browsing through t= he site and took a look at the component reports. Checkstyle alone seems cl= ose to pointless as there are over 200 errors in log4j-api alone. log4j-cor= e has over 2000 errors. Even new files that were formatted with our formatt= er settings such as the CassandraAppender plugin have import ordering error= s. I also disagree with some of the rules configured, but that doesn't = really matter when we don't enforce it in the first place.

Anyways, what's the point of configuring this and adding checkst= yle comments yet not even using it? The only project I've come across i= n the wild so far that has checkstyle configured properly was Spring Boot, = and your pull request has to pass the checkstyle check to even be mergeable= .

Perhaps if we wish to actually use it, we could = loosen the rules down to a much smaller set that actually matches the forma= tter settings in src/ide/. If the rules matched our code base, then we coul= d also have Jenkins run checkstyle checks which would keep us informed when= we mess up, and it would also be useful for pull requests (I've had to= reformat many patches in the past).

Related, ther= e's the style guide <https://logging.apache.org/log4j/2.= x/javastyle.html> which I'm pretty sure I've never even look= ed at before. This could also be normalized with our formatter files. I'= ;ve generally thought of our code style summarized as:

=
* 4 space indent
* use final
* no star imports out= side tests (and those should generally be static imports)
* impor= ts should be in some sort of alphabetical order (this is really difficult t= o match between IntelliJ and Eclipse for some reason; I've had rather o= bnoxious fights about this in the past thanks to import-order-induced merge= conflicts)
* try to stick to unix line endings, but we're ra= ther mixed still
* every file needs a license header unless it= 9;s impossible to include comments
* use CamelCaseClassNames, eve= n for acronyms
* no hungarian notation or other distracting namin= g conventions
* otherwise stick to typical Sun style that everyon= e basically recognizes (that the JDK doesn't even use itself)

--=C2=A0
Matt Sicker &= lt;boards@gmail.com>




--= =C2=A0
= 3D"MagineTV"=C2=A0
Privileged and/or Confidential Information may be contained in this m= essage. If you are not the addressee indicated in this message
(or respo= nsible for delivery of the message to such a person), you may not copy or d= eliver this message to anyone. In such case,=C2=A0
you should destroy th= is message and kindly notify the sender by reply email. =C2=A0=C2=A0
<= /div>



--=C2=A0
Matt Sicker = <boards@gmail.com<= /a>>





--
=
Matt Sick= er <boards@gmail.c= om>
--f4030438f198b6432605482f4caa--