From log4j-dev-return-49752-apmail-logging-log4j-dev-archive=logging.apache.org@logging.apache.org Fri Feb 10 19:01:37 2017 Return-Path: X-Original-To: apmail-logging-log4j-dev-archive@www.apache.org Delivered-To: apmail-logging-log4j-dev-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 1297519613 for ; Fri, 10 Feb 2017 19:01:37 +0000 (UTC) Received: (qmail 23095 invoked by uid 500); 10 Feb 2017 19:01:36 -0000 Delivered-To: apmail-logging-log4j-dev-archive@logging.apache.org Received: (qmail 23052 invoked by uid 500); 10 Feb 2017 19:01:36 -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 23042 invoked by uid 99); 10 Feb 2017 19:01:36 -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 19:01:36 +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 67956C03B1 for ; Fri, 10 Feb 2017 19:01:36 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd1-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: 5.4 X-Spam-Level: ***** X-Spam-Status: No, score=5.4 tagged_above=-999 required=6.31 tests=[HTML_MESSAGE=2, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_SORBS_SPAM=0.5, RCVD_IN_SORBS_WEB=3.6] autolearn=disabled 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 SPY4Q4d-qjhe for ; Fri, 10 Feb 2017 19:01:34 +0000 (UTC) Received: from smtp687.redcondor.net (smtp687.redcondor.net [208.80.206.87]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with ESMTPS id 30F3E5F647 for ; Fri, 10 Feb 2017 19:01:34 +0000 (UTC) Received: from mailproxy10.neonova.net ([137.118.22.75]) by smtp687.redcondor.net ({f50aeeea-7a14-4dd3-bdda-1246754c15b3}) via TCP (outbound) with ESMTP id 20170210190129426_0687 for ; Fri, 10 Feb 2017 19:01:29 +0000 X-RC-FROM: X-RC-RCPT: Received: from [10.9.100.106] (unknown [208.93.128.118]) (Authenticated sender: ralph.goers@dslextreme.com) by mailproxy10.neonova.net (Postfix) with ESMTPA id ACB9D360165 for ; Fri, 10 Feb 2017 14:01:25 -0500 (EST) From: Apache Content-Type: multipart/alternative; boundary="Apple-Mail=_9D87DF72-55EC-43B6-8E3F-AFB5CCAC6F7F" Mime-Version: 1.0 (Mac OS X Mail 10.2 \(3259\)) Subject: Re: Regarding Checkstyle, PMD, and formatting Date: Fri, 10 Feb 2017 12:01:24 -0700 References: <19A44C71-5241-48D4-936E-8B04BB22BFCA@dslextreme.com> To: Log4J Developers List In-Reply-To: Message-Id: X-Mailer: Apple Mail (2.3259) X-DLP-OUTBOUND: 137.118.22.64/27 X-MAG-OUTBOUND: greymail.redcondor.net@137.118.22.64/27 --Apple-Mail=_9D87DF72-55EC-43B6-8E3F-AFB5CCAC6F7F Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=utf-8 Well, smalltalk might be nicer but we code in Java. I prefer to always = use parens unless all the operators are the same. I=E2=80=99m old so I = can never remember the precedence order. Ralph > On Feb 10, 2017, at 11:32 AM, Gary Gregory = wrote: >=20 > On Fri, Feb 10, 2017 at 8:07 AM, Matt Sicker > wrote: > The "unnecessary parenthesis" rule is somewhat annoying. While it has = good intentions, it'll also flag something like this: >=20 > foo || (bar && baz) >=20 > Sure, && has higher precedence than || (had to look it up just now), = but who can remember those kinds of rules anyways? >=20 > It's one of the many things Smalltalk got right. Left to right, simple = as that. Different, yes, but simple. >=20 > Gary > =20 >=20 > On 10 February 2017 at 09:36, Remko Popma > wrote: > +1 on braces=20 >=20 > 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=99= t, Gary will fix it ;-) >=20 > 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. >=20 > Ralph >=20 >> On Feb 10, 2017, at 8:09 AM, Matt Sicker > wrote: >>=20 >> At work, I've switched from final everywhere to final everywhere but = local variables while maintaining effective finality instead. I just = wish Java had final be the default. >>=20 >> 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.=20 >>=20 >> Otherwise all looks reasonable.=20 >>=20 >> Sent from my iPhone >>=20 >> On Feb 10, 2017, at 17:55, Mikael St=C3=A5ldal = > wrote: >>=20 >>> Seems reasonable. >>>=20 >>> On Fri, Feb 10, 2017 at 5:56 AM, Gary Gregory = > wrote: >>> I agree with all that! :-) >>>=20 >>> Gary >>>=20 >>>=20 >>> 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 200 = errors in log4j-api alone. log4j-core has over 2000 errors. Even new = files 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 don't enforce it in the first place. >>>=20 >>> 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 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. >>>=20 >>> 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 mess = up, and it would also be useful for pull requests (I've had to reformat = many patches in the past). >>>=20 >>> Related, there's the style guide = > which I'm pretty = sure I've never even looked at before. This could also be normalized = with our formatter files. I've generally thought of our code style = summarized as: >>>=20 >>> * 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) >>>=20 >>> --=20 >>> Matt Sicker > >>>=20 >>>=20 >>>=20 >>>=20 >>> --=20 >>> =20 >>>=20 >>> Mikael St=C3=A5ldal >>> Senior software developer=20 >>>=20 >>> Magine TV >>> mikael.staldal@magine.com =20 >>> Grev Turegatan 3 | 114 46 Stockholm, Sweden | www.magine.com=C2=A0= >>>=20 >>> 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,=20 >>> you should destroy this message and kindly notify the sender by = reply email. =20 >>=20 >>=20 >>=20 >> --=20 >> Matt Sicker > >=20 >=20 >=20 >=20 >=20 > --=20 > Matt Sicker > >=20 >=20 >=20 > --=20 > E-Mail: garydgregory@gmail.com | = ggregory@apache.org=C2=A0 > Java Persistence with Hibernate, Second Edition = =C2=A0 = > JUnit in Action, Second Edition = =C2=A0 = > Spring Batch in Action = =C2=A0 = > Blog: http://garygregory.wordpress.com = =20 > Home: http://garygregory.com/ > Tweet! http://twitter.com/GaryGregory --Apple-Mail=_9D87DF72-55EC-43B6-8E3F-AFB5CCAC6F7F Content-Transfer-Encoding: quoted-printable Content-Type: text/html; charset=utf-8
Well, smalltalk might be nicer but we code in = Java. I prefer to always use parens unless all the operators are the = same. I=E2=80=99m old so I can never remember the precedence = order.

Ralph

On Feb 10, 2017, at 11:32 AM, Gary Gregory = <garydgregory@gmail.com> wrote:

On Fri, Feb 10, 2017 at = 8:07 AM, Matt Sicker <boards@gmail.com> wrote:
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?

It's one of the many things Smalltalk got right. Left to = right, simple as that. Different, yes, but simple.

Gary
 

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

On Sat, Feb 11, 2017 at 12:35 AM, = Apache <ralph.goers@dslextreme.com> wrote:
You don=E2=80=99= t 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 <boards@gmail.com> wrote:

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

On 10 = February 2017 at 05:34, Remko Popma <remko.popma@gmail.com> 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 <mikael.staldal@magine.com> wrote:

Seems reasonable.

On Fri, = Feb 10, 2017 at 5:56 AM, Gary Gregory <garydgregory@gmail.com> wrote:
I agree with all that! :-)

Gary


On Feb 9, 2017 7:05 PM, "Matt = Sicker" <boards@gmail.com> 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 200 errors in log4j-api alone. log4j-core has over 2000 errors. = Even new files 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 don'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 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 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 mess up, and it would also be useful for = pull requests (I've had to reformat many patches in the past).

Related, there's the = style guide <https://logging.apache.org/log4j/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've 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 <boards@gmail.com>
<= /div>



-- 
3D"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 <boards@gmail.com>




-- 
Matt Sicker <boards@gmail.com>
=



-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org 
Java Persistence with = Hibernate, Second Edition 
JUnit in Action, Second = Edition 
Spring Batch in Action 
Blog: http://garygregory.wordpress.com 
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

= --Apple-Mail=_9D87DF72-55EC-43B6-8E3F-AFB5CCAC6F7F--