search

checkstyle / checkstyle

Checkstyle is a development tool to help programmers write Java code that adheres to a coding standard. By default it supports the Google Java Style Guide and Sun Code Conventions, but is highly configurable. It can be invoked with an ANT task and a command line program.
https://checkstyle.org
GNU Lesser General Public License v2.1
8.32k stars 3.66k forks source link

DefaultLogger & XMLLogger shall be closed in place where is opened #883

Open damianszczepanik opened 9 years ago

damianszczepanik commented 9 years ago

In Main class XMLLogger and DefaultLogger are created with constructor that takes boolean value to decide whether the output stream shall be closed or not.

It's better to control if the stream shall be closed or not from method where the stream was created. In that case we avoid passing boolean value into loggers.

Also consider to create AbstractLogger that may keep PrintWritter attribute and method for closing this stream. Currently code for closing stream is duplicated and I'm not sure if stream is closed when exception is thrown so auditFinished() may not be invoked.

romani commented 9 years ago

I do not like that option too, but lets wait till refactoring of #596 is done.

romani commented 9 years ago

fix for #67 was revered as it brake compatibility with maven plugin that we use for reports generation.

Any changes to constructors signature have to be postponed till maven plugin switch to recent versions (right now it stands on last version that use java6).

So this issue is postponed till maven plugin switch to java7 binaries.

romani commented 9 years ago

just an idea to implement that quicker then maven plugin will use latest version:

All we need is to keep signature of c-tor to be the same, we could review sources of plugin, if he do not use that argument with variable and only with a constant "true" or "false" , we could keep a c-tor signature but internally do what we need.

rnveach commented 7 years ago

eclipse-cs uses a custom listener called CheckstyleAuditListener. maven plugin uses those constructors with default value of true.

We will have to keep that signature unless we want to break compatibility with them (like in CS 8).

romani commented 7 years ago

Marked as checkstyle8