HPCC4J-791 Add automated code formatting - #978
Conversation
- Add formatter-maven-plugin to pom.xml with validate/format goals - Enhance copilot-instructions.md with detailed Eclipse formatting enforcement rules - Fix 8 critical Eclipse formatter settings to minimize disruptive changes: - Add Code Formatting section to README.md with usage examples - Enable 'mvn formatter:validate' for CI and 'mvn formatter:format' for auto-fixing" Signed-off-by: Rodrigo Pastrana <rodrigo.pastrana@lexisnexisrisk.com>
|
Jira Issue: https://hpccsystems.atlassian.net/browse/HPCC4J-791 Jirabot Action Result: |
🔄 Upmerge Test ResultsStatus: ✅ All branches merged successfully ✅ Successful Branches (5)
|
There was a problem hiding this comment.
Pull request overview
Adds an automated Java formatting workflow (Eclipse formatter config + Maven plugin + docs) so formatting can be validated in CI and applied locally.
Changes:
- Adds
formatter-maven-pluginconfiguration to the parentpom.xml - Tweaks several Eclipse formatter settings to reduce disruptive reformat diffs
- Documents formatter usage in
README.mdand adds detailed formatting guidance in.github/copilot-instructions.md
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pom.xml | Introduces formatter-maven-plugin and binds an execution to the Maven lifecycle |
| eclipse/HPCC-JAVA-Formatter.xml | Updates several Eclipse formatter settings (alignment/brace/newline behaviors) |
| README.md | Adds a “Code Formatting” section with formatter:validate / formatter:format usage |
| .github/copilot-instructions.md | Expands formatting rules and review expectations for contributors |
| <plugin> | ||
| <groupId>net.revelc.code.formatter</groupId> | ||
| <artifactId>formatter-maven-plugin</artifactId> | ||
| <version>${formatter.maven.plugin.version}</version> | ||
| <configuration> | ||
| <configFile>${maven.multiModuleProjectDirectory}/eclipse/HPCC-JAVA-Formatter.xml</configFile> | ||
| <encoding>UTF-8</encoding> | ||
| <lineEnding>LF</lineEnding> | ||
| <directories> | ||
| <directory>${project.basedir}/src/main/java</directory> | ||
| <directory>${project.basedir}/src/test/java</directory> | ||
| </directories> | ||
| <excludes> | ||
| <!-- Exclude generated code --> | ||
| <exclude>**/gen/**</exclude> | ||
| <exclude>**/ws/client/gen/**</exclude> | ||
| <exclude>**/wrappers/gen/**</exclude> | ||
| <exclude>**/antlr/**/*.java</exclude> | ||
| </excludes> | ||
| </configuration> | ||
| <executions> | ||
| <execution> | ||
| <id>format-check</id> | ||
| <goals> | ||
| <goal>format</goal> | ||
| </goals> | ||
| <phase>validate</phase> | ||
| </execution> | ||
| </executions> | ||
| </plugin> |
There was a problem hiding this comment.
The execution is bound to the validate phase but runs the format goal, which will modify sources during normal builds/CI (e.g., mvn test), rather than failing the build on violations. To match the PR intent (validate in CI, format only when explicitly invoked), bind the validate goal to the validate phase, and keep format as a developer-invoked goal (or gate it behind an opt-in profile/property).
|
|
||
| This project uses the Eclipse formatter configuration (`eclipse/HPCC-JAVA-Formatter.xml`) to ensure consistent Java code formatting. You can format code using Maven commands: | ||
|
|
||
| **Validate formatting** (check for violations without modifying files): |
There was a problem hiding this comment.
The docs describe formatter:validate as the CI-friendly check, but the current pom.xml binds formatter:format to the Maven validate phase, meaning a standard build will auto-modify files. Either update the pom.xml execution to run validate (preferred) or adjust this documentation to reflect that formatting occurs automatically during the lifecycle.
| - **Blank lines**: | ||
| - 1 line after package declaration | ||
| - 1 line after import section | ||
| - 1 line between methods |
There was a problem hiding this comment.
This guidance conflicts with the updated Eclipse formatter setting blank_lines_before_method (now 0 in eclipse/HPCC-JAVA-Formatter.xml), so Copilot would be instructed to flag/require something the formatter won’t enforce. Please align these bullet points with the actual formatter configuration (or revert the formatter change if 1 blank line between methods is the desired standard).
| - 1 line between methods | |
| - Follow Eclipse formatter configuration for blank lines between methods (currently 0 configured blank lines before methods) |
jpmcmu
left a comment
jpmcmu
left a comment
There was a problem hiding this comment.
@rpastrana Looks good to me
Type of change:
Custom Platform Testing (Optional)
Custom HPCC-Platform Repository:
repository:
Custom HPCC-Platform Branch:
branch:
Checklist:
Testing: