feat: add execution ID parameter to failing spotless check log#2483
feat: add execution ID parameter to failing spotless check log#2483dpolivaev wants to merge 3 commits intodiffplug:mainfrom
Conversation
…ror messaging, issue diffplug#1988 - Introduced a new parameter `executionId` to capture the execution ID from Maven's configuration. - Updated error messages to include the execution ID when suggesting commands to fix format violations. - Enhanced test cases to validate the new behavior of error messages based on the execution ID.
…r format violations (diffplug#1988)
| * The execution ID from Maven's configuration. | ||
| */ | ||
| @Parameter(defaultValue = "${mojoExecution.executionId}") | ||
| private String executionId; |
There was a problem hiding this comment.
The execution ids of different goals usually differ as well!
There was a problem hiding this comment.
This is how we use it:
<profile>
<id>palantir-java-format-profile</id>
<activation>
<jdk>[11,)</jdk>
</activation>
<build>
<plugins>
<plugin>
<groupId>com.diffplug.spotless</groupId>
<artifactId>spotless-maven-plugin</artifactId>
<executions>
<execution>
<!--
Run "mvn spotless:check@palantir-java-format" to check the
formatting outside of the "mvn verify" phase.
Similar: To format your code, run "mvn spotless:apply@palantir-java-format".
You need to add the execution id with "@palantir-java-format" to your comand,
so spotless will find its configuration containing the palantirJavaFormat.
-->
<id>palantir-java-format</id>
<phase>verify</phase>
<goals>
<goal>check</goal>
</goals>
<configuration>
<skip>${spotless.skip.palantirJavaFormat}</skip>
<java>
<palantirJavaFormat/>
</java>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</profile>Therefore we do not need different configurations and different execution IDs for the different goals (check and apply). We don't repeat ourselves.
There was a problem hiding this comment.
They can be the same but don’t have to. With the default assuming the same there is a high chance something is emitted which does not work. I would leave out the default value for this parameter
There was a problem hiding this comment.
Do you think we need an optional separate parameter with a dedicated value? How would you name it?
The whole idea is to output correct command line automatically. Currently it outputs just run 'mvn spotless:apply' and it was the reason why I created the pull request.
There was a problem hiding this comment.
I think just changing the output inncase a dedicated execution id is configured is totally sufficient
…Mojo - Updated the logic to check for execution IDs, ensuring that messages correctly suggest commands based on the presence of default execution IDs. - Added a new test case to validate behavior when no explicit execution ID is provided, confirming that the output message is simplified accordingly.
|
Maven always adds an execution id even if it is not configured in the pom file. |
|
This needs a |
|
I'm open to merging this, but it needs to pass CI, and it needs signoff from an existing maven contributor. |
for improved error messaging, issue #1988
executionIdto capture the execution ID from Maven's configuration.