Skip to content

Conversation

@CharlesDuboisSAP
Copy link
Contributor

@CharlesDuboisSAP CharlesDuboisSAP commented Feb 6, 2025

Context

mvn install should fail as fast as possible regardless of what step fails.

Feature scope:

  • Organized plugins by phase (cosmetic)
  • Moved SpotBugs to run after Jacoco (it never fails and takes a while, it is now in last)

@CharlesDuboisSAP CharlesDuboisSAP added the please-review Request to review a pull-request label Feb 6, 2025
@CharlesDuboisSAP CharlesDuboisSAP self-assigned this Feb 6, 2025
@CharlesDuboisSAP CharlesDuboisSAP changed the title chore: CI fail early chore: mvn install fail early Feb 6, 2025
<artifactId>maven-resources-plugin</artifactId>
<version>3.3.1</version>
<groupId>com.github.spotbugs</groupId>
<artifactId>spotbugs-maven-plugin</artifactId>
Copy link
Contributor Author

@CharlesDuboisSAP CharlesDuboisSAP Feb 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SpotBugs is last after Jacoco

@CharlesDuboisSAP CharlesDuboisSAP enabled auto-merge (squash) February 6, 2025 14:33
@CharlesDuboisSAP CharlesDuboisSAP changed the title chore: mvn install fail early chore: DevOps mvn install fail early Feb 6, 2025
MatKuhr
MatKuhr previously requested changes Feb 7, 2025
Copy link
Member

@MatKuhr MatKuhr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not suitable when doing PoCs or larger refactorings. There we don't want to fix all PMD and Javadoc requirements just to compile.

Instead, if you want to run all checks fast (i.e. before/without tests), just run mvn install -DskipTests or create an alias for that. Compile takes around 10s (on my machine), so there is no point in moving checks to before that for speed

@CharlesDuboisSAP
Copy link
Contributor Author

This is not suitable when doing PoCs or larger refactorings. There we don't want to fix all PMD and Javadoc requirements just to compile.

CheckStyle and PMD still run after compile.

@MatKuhr
Copy link
Member

MatKuhr commented Feb 7, 2025

Okay fair, but still it's beneficial to run tests without fixing all javadoc warnings, why not just mvn install -DskipTests? our CI/CD pipeline does that already anyway..

@CharlesDuboisSAP
Copy link
Contributor Author

Okay fair, but still it's beneficial to run tests without fixing all javadoc warnings, why not just mvn install -DskipTests? our CI/CD pipeline does that already anyway..

I run tests with IntelliJ, the only time I run mvn install is to make sure my PR is green

<goals>
<goal>prepare-agent</goal>
</goals>
<phase>initialize</phase>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this change do?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's redundant, the default is already initialize

Copy link
Member

@MatKuhr MatKuhr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@CharlesDuboisSAP CharlesDuboisSAP merged commit 3b3dc57 into main Feb 7, 2025
7 checks passed
@CharlesDuboisSAP CharlesDuboisSAP deleted the coverage branch February 7, 2025 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

please-review Request to review a pull-request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants