-
Notifications
You must be signed in to change notification settings - Fork 9
Add formatter plugins and automatic format execution profile #41
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,4 +23,6 @@ CosServices.cfg | |
| audit.log | ||
| *.zip | ||
| *.db | ||
| *.json | ||
| *.json | ||
| .cache | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -56,6 +56,7 @@ | |
| <buildproperty.date>${maven.build.timestamp}</buildproperty.date> | ||
| <!-- Checkstyle is disabled by default, but it could be enabled for default by a particular module --> | ||
| <checkstyle.skip>false</checkstyle.skip> | ||
| <format.skip>false</format.skip> | ||
| <ipv6.server.jvm.args>-Djboss.bind.address=[::1] -Djboss.bind.address.management=[::1] | ||
| -Djboss.bind.address.unsecure=[::1] -Djava.net.preferIPv4Stack=false -Djava.net.preferIPv6Addresses=true</ipv6.server.jvm.args> | ||
| <jvm.args.byteman>-Dorg.jboss.byteman.verbose | ||
|
|
@@ -98,6 +99,7 @@ | |
| <version.com.github.spotbugs.spotbugs-maven-plugin>4.8.3.1</version.com.github.spotbugs.spotbugs-maven-plugin> | ||
| <version.com.h2database>2.2.224</version.com.h2database> | ||
| <version.doxia>1.8.1</version.doxia> | ||
| <version.formatter.plugin>2.26.0</version.formatter.plugin> | ||
|
|
||
| <version.hamcrest>2.2</version.hamcrest> | ||
| <version.httpcomponents>4.5.14</version.httpcomponents> | ||
|
|
@@ -705,24 +707,6 @@ | |
| </execution> | ||
| </executions> | ||
| </plugin> | ||
| <plugin> | ||
| <groupId>com.github.ekryd.sortpom</groupId> | ||
| <artifactId>sortpom-maven-plugin</artifactId> | ||
| <version>${version.sortpom}</version> | ||
| <configuration> | ||
| <skip>${sortpom.skip}</skip> | ||
| <createBackupFile>false</createBackupFile> | ||
| <sortProperties>true</sortProperties> | ||
| </configuration> | ||
| <executions> | ||
| <execution> | ||
| <goals> | ||
| <goal>sort</goal> | ||
| </goals> | ||
| <phase>verify</phase> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
| <plugin> | ||
| <groupId>org.apache.maven.plugins</groupId> | ||
| <artifactId>maven-surefire-plugin</artifactId> | ||
|
|
@@ -753,6 +737,25 @@ | |
| </execution> | ||
| </executions> | ||
| </plugin> | ||
| <plugin> | ||
| <groupId>net.revelc.code.formatter</groupId> | ||
| <artifactId>formatter-maven-plugin</artifactId> | ||
| <version>${version.formatter.plugin}</version> | ||
| <configuration> | ||
| <!-- store outside of target to speed up formatting when mvn clean is used --> | ||
| <cachedir>.cache/formatter-maven-plugin-${version.formatter.plugin}</cachedir> | ||
| <configFile>eclipse-format.xml</configFile> | ||
| <lineEnding>LF</lineEnding> | ||
| <skip>${format.skip}</skip> | ||
| </configuration> | ||
| <dependencies> | ||
| <dependency> | ||
| <groupId>io.quarkus</groupId> | ||
| <artifactId>quarkus-ide-config</artifactId> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Overall the formatting changes look good, thanks @xstefank
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is a great idea, yeah. In my opinion we can decouple it later and not need Martin to change that part now. I do think that we should have the CONTRIBUTING.md reference the expectation (citing a reference to the quarkus one) with this pull request (and we change that later too) so it goes in as close to atomic change as possible but if needed the CONTRIBUTING.md change could be added as a separate commit but we should be ready to go with that.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 |
||
| <version>3.22.2</version> | ||
| </dependency> | ||
| </dependencies> | ||
| </plugin> | ||
| </plugins> | ||
| </build> | ||
| <profiles> | ||
|
|
@@ -999,5 +1002,64 @@ | |
| </plugins> | ||
| </build> | ||
| </profile> | ||
| <profile> | ||
| <id>format</id> | ||
| <activation> | ||
| <activeByDefault>true</activeByDefault> | ||
| <property> | ||
| <name>!no-format</name> | ||
| </property> | ||
| </activation> | ||
| <build> | ||
| <plugins> | ||
| <plugin> | ||
| <groupId>net.revelc.code.formatter</groupId> | ||
| <artifactId>formatter-maven-plugin</artifactId> | ||
| <executions> | ||
| <execution> | ||
| <phase>process-sources</phase> | ||
| <goals> | ||
| <goal>format</goal> | ||
| </goals> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
| <plugin> | ||
| <groupId>net.revelc.code</groupId> | ||
| <artifactId>impsort-maven-plugin</artifactId> | ||
| <executions> | ||
| <execution> | ||
| <id>sort-imports</id> | ||
| <goals> | ||
| <goal>sort</goal> | ||
| </goals> | ||
| </execution> | ||
| </executions> | ||
| <configuration> | ||
| <removeUnused>true</removeUnused> | ||
| </configuration> | ||
| </plugin> | ||
| <plugin> | ||
| <groupId>com.github.ekryd.sortpom</groupId> | ||
| <artifactId>sortpom-maven-plugin</artifactId> | ||
| <version>${version.sortpom}</version> | ||
| <configuration> | ||
| <skip>${sortpom.skip}</skip> | ||
| <createBackupFile>false</createBackupFile> | ||
| <sortProperties>true</sortProperties> | ||
| </configuration> | ||
| <executions> | ||
| <execution> | ||
| <goals> | ||
| <goal>sort</goal> | ||
| </goals> | ||
| <phase>verify</phase> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
| </plugins> | ||
| </build> | ||
| </profile> | ||
|
|
||
| </profiles> | ||
| </project> | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I might be "wrong" about where I am going with this - but I noticed this part and wonder if we know that the eclipse format (or maybe rather than a style, it means formating for Eclipse IDE?) is compatible with https://github.com/jbosstm/narayana-checkstyle-config/blob/main/src/main/resources/narayana-checkstyle/checkstyle.xml. Hopefully it is, but we should anyway add an update to the https://github.com/jbosstm/lra/blob/main/CONTRIBUTING.md to describe this formatting requirement too (assuming it is a new one). Like Mike though, I defer to @marcosgopen for the review (but would mention I could imagine sharing this proposed addition to the developer "requirements" on Zulip (for feedback) and merging say a week or so later if there are no blockers raised in the wider community after the chance for feedback but again, I defer to Marco for deciding this.