aherbert commented on pull request #22:
URL: https://github.com/apache/commons-beanutils/pull/22#issuecomment-682619129
I've just had a look at this. In the current config it reformats a lot of
javadoc that has been laid out for readability. For example:
```
* The main features of this implementation are:
* <ul>
* <li><b>Element Conversion</b> - delegates to a {@link Converter},
* appropriate for the type, to convert individual elements
* of the array. This leverages the power of existing converters
* without having to replicate their functionality for converting
* to the element type and removes the need to create a specific
* array type converters.</li>
* <li><b>Arrays or Collections</b> - can convert from either arrays or
* Collections to an array, limited only by the capability
* of the delegate {@link Converter}.</li>
```
This all gets the leading whitepsace trimmed which I think is less readable.
It also indents wrapped lines a lot (using 12 spaces). So something that has
been formatted to be human readable
```java
if (value instanceof Number ||
value instanceof Boolean ||
value instanceof java.util.Date) {
```
Becomes:
```java
if (value instanceof Number ||
value instanceof Boolean ||
value instanceof java.util.Date) {
```
Other examples of human readable whitespace:
```java
private static final char NESTED = '.';
private static final char MAPPED_START = '(';
private static final char MAPPED_END = ')';
private static final char INDEXED_START = '[';
private static final char INDEXED_END = ']';
```
to
```java
private static final char NESTED = '.';
private static final char MAPPED_START = '(';
private static final char MAPPED_END = ')';
private static final char INDEXED_START = '[';
private static final char INDEXED_END = ']';
```
Also when I run:
```
mvn formatter:format
mvn checkstyle:checkstyle
```
I still have some errors from checkstyle. The checkstyle plugin config is
not present in the `<build>` section so I will fix this in master. When I fix
this configuration I get 35 errors before the formatter runs and 88 after
(mainly trailing whitespace in the javadocs). If I disable javadoc formatting
there are 10 errors. If I include the test sources there are 51 errors before
the formatter and 13 after (with no formatting of javadoc). Adding the test
sources requires the following to be added to the formatter config:
```
<directory>${project.build.testSourceDirectory}</directory>
```
So it is fixing some things but not all. I think the errors checkstyle
reports are valid and are things that the formatter cannot (or won't) change so
we can fix these after. Most are line length issues where the checkstyle
suppressions need to be added to ignore the lines inside `@formatter:off` tags.
Some are genuine errors that should be fixed.
Can you rebase on master to pick up the checkstyle config and have an
investigation of the javadoc formatting and line indentation. My preference
would be to turn off javadoc formatting and change the indentation rules to not
use such a big indentation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
