Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Formatted source code using Apache Commons standards. #23

Closed
wants to merge 5 commits into from
Closed

Formatted source code using Apache Commons standards. #23

wants to merge 5 commits into from

Conversation

melloware
Copy link
Contributor

Using the Eclipse formatter provided in the previous PR.

@aherbert
Copy link
Contributor

Although this a good step to standardisation I wondered if there is a way to achieve the same in a maven plugin so that those not using eclipse can be compliant?

A quick check found this formatter-maven-plugin which is built on top of the eclipse formatter. Could you have a look and see if it works when added to the pom for the code in this PR. I.e. you should be able to run mvn formatter:format and it should not change any of your files that you formatted in eclipse.

If it works then the formatter:validate goal could be added to the goals run by the CI server (in a future PR) to make sure new code has not forgotten to run the formatting.

@melloware
Copy link
Contributor Author

@aherbert Let me try that and I will report back.

@melloware
Copy link
Contributor Author

@aherbert it worked like a charm. In fact it picked up directories I missed because I assumed Eclipse recursed directories when it formatted but it did not. but the Plugin does and picks up the whole source tree.

@aherbert
Copy link
Contributor

I think that the eclipse formatter will recurse the directory if you start at a resource folder (e.g. src/main/java). But if you start at a package in the directory tree below the top level (e.g. org.apache.commons.beautils2) then it only processes those java files in the current package.

Have you looked at how the formatter treated comments. I see that you have configured it to allow formatting in javadoc comments. In my experience this is not good for compact lists:

  /**
   * Test method.
   *
   * <ul>
   *  <li>This
   *  <li>list
   * </ul>
   */

Becomes:

  /**
   * Test method.
   *
   * <ul> <li>This <li>list </ul>
   */

The javadoc will still render the same but anyone reading the source file will struggle.

So I would recommend you have a scan through the code for <li> elements and check them.

You can solve this by adding spaces:

  /**
   * Test method.
   *
   * <ul>
   *
   *  <li>This
   *
   *  <li>list
   *
   * </ul>
   */

becomes:

  /**
   * Test method.
   *
   * <ul>
   *
   * <li>This
   *
   * <li>list
   *
   * </ul>
   */

Basically just stripping leading whitespace.

Also check what it has done in <pre> blocks. You can protect them to some extent using <pre>{@code but then the code cannot contain } which is hard to avoid if documenting loops and other block statements.

@melloware
Copy link
Contributor Author

melloware commented Mar 20, 2020

I tested

 /**
     * Test method.
     *
     * <ul>
     *     <li>This
     *     <li>list
     * </ul>
     */

became this...

    /**
     * Test method.
     * <ul>
     * <li>This
     * <li>list
     * </ul>
     */

@melloware
Copy link
Contributor Author

Closing this PR. Will create a new one

@melloware melloware closed this May 31, 2020
@melloware melloware deleted the BU-FORMATTED branch May 31, 2020 11:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants