mccheah opened a new pull request #143: Applies the Baseline plugin for iceberg-api only. URL: https://github.com/apache/incubator-iceberg/pull/143 Part of #24. Replaces #28. Previously, whenever styles were inconsistent across the Iceberg codebase, it was up to code reviewers to catch the inconsistencies and to catch them before merging. This is prone to error, and little code linting problems can add up over time. [Baseline](https://github.com/palantir/gradle-baseline) is a code linting toolkit for Gradle. It consists of multiple submodules that when taken together allow automation to enforce consistent coding guidelines. For more information, refer to the Baseline docs. As a proof of concept, the full Baseline suite minus spotless-java is now only applied for the iceberg-api project, while IntelliJ project configuration is applied for all projects. We apply the Baseline linting changes only to `iceberg-api` to minimize code churn and to show how we can introduce baseline checks incrementally. Eventually we can add the same tooling to all the submodules. Baseline's coding conventions are inherited from the Google Style Guide. For more information on the style rules that are given out of the box, refer to [this documentation](https://github.com/palantir/gradle-baseline/blob/develop/docs/java-style-guide/readme.md). There were a number of style conventions that we've adopted from Baseline that were not previously enforced in the project. A subset of them are listed as follows. * We no longer allow parameter names and local variable names to hide the names of fields. So if a field is named name, no parameters of any methods in the class can be called name. * All field, parameter, and variable names must be more than 1 character long. So `W` is no longer a valid field name in `Truncate`, for example. There were a lot of cases like this. * All fields must be private with accessor methods. I believe in one case this affects a public API. * Utility classes must have a private no-arg constructor. * All unnecessary parentheses are disallowed. * `Preconditions` must use a constant format string. Variance in what the preconditions message would produce must be given as format string arguments. * Overloaded methods must be grouped together. * Use Java's `StandardCharsets` instead of Guava's `Charsets`. There's a number of Baseline-opinionated defaults that we don't adopt here to reduce the code churn and also just because Iceberg appears to hold different opinions. Here's some of them: * Baseline's spotless plugin doesn't work when we allow static imports, so we just disable it entirely. * Indentation is still 2 spaces with 4 space continuation indent. * No opinions are held about Javadoc formatting (should we set a standard for this?) * We allow a larger variety of static imports, though by default static imports are still forbidden * Static imports are listed after non-static imports, not before * Cyclomatic complexity is bumped to 12, up from 10 * We allow some more abbreviations - particularly `UUID`, `ID`, `IO`, and `UTC`. In one case we allow a method to be called `flipLR` by applying a `SuppressWarnings` annotation on the method directly. * No opinions are held about the assertion methods we use for tests.
---------------------------------------------------------------- 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] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
