Skip to content

Do not execute spotless in Java 21#11670

Merged
Jackie-Jiang merged 1 commit intoapache:masterfrom
gortiz:spotless-improvements
Sep 25, 2023
Merged

Do not execute spotless in Java 21#11670
Jackie-Jiang merged 1 commit intoapache:masterfrom
gortiz:spotless-improvements

Conversation

@gortiz
Copy link
Copy Markdown
Contributor

@gortiz gortiz commented Sep 25, 2023

The reason to be of this PR is to be able to progress on #11656.

Spotless does not support Java 21 yet (see diffplug/spotless#1822) so this PR moves the spotless usage to its own profile, which is active automatically if and only if the JVM being used is not 21. Once Spotless support Java 21 (probably when diffplug/spotless#1822 is merged) we could just remove this profile and move the spotless config to the build->pluginManagement section.

Why don't just disable spotless with -Dspotless.skip?

IT would be a great, but spotless doesn't work in that way. Even with that property set, spotless actually executes. What the flag controls is whether the pipeline should fail or not if spotless detects an issue in the code (like importing something that is not being used). When using Java 21, spotless throws an exception that is not caught, so it always aborts the maven execution, even if -Dspotless.skip is set.

This changes are included in #11672, so we can decide to merge this alone or merge everything together.

@gortiz gortiz mentioned this pull request Sep 25, 2023
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Sep 25, 2023

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 14.45%. Comparing base (e6655dd) to head (2cb9bbb).
⚠️ Report is 4301 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (e6655dd) and HEAD (2cb9bbb). Click for more details.

HEAD has 36 uploads less than BASE
Flag BASE (e6655dd) HEAD (2cb9bbb)
unittests 6 1
unittests2 3 1
temurin 12 3
java-17 4 0
integration 6 2
integration2 3 1
java-20 4 0
java-11 4 3
integration1 3 1
unittests1 3 0
Additional details and impacted files
@@              Coverage Diff              @@
##             master   #11670       +/-   ##
=============================================
- Coverage     63.10%   14.45%   -48.66%     
+ Complexity     1121      201      -920     
=============================================
  Files          2343     2343               
  Lines        125693   125691        -2     
  Branches      19310    19310               
=============================================
- Hits          79315    18163    -61152     
- Misses        40728   105991    +65263     
+ Partials       5650     1537     -4113     
Flag Coverage Δ
integration <0.01% <ø> (ø)
integration1 <0.01% <ø> (ø)
integration2 0.00% <ø> (ø)
java-11 14.45% <ø> (-48.60%) ⬇️
java-17 ?
java-20 ?
temurin 14.45% <ø> (-48.66%) ⬇️
unittests 14.44% <ø> (-48.66%) ⬇️
unittests1 ?
unittests2 14.44% <ø> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Jackie-Jiang Jackie-Jiang added the testing Related to tests or test infrastructure label Sep 25, 2023
@Jackie-Jiang Jackie-Jiang merged commit e144147 into apache:master Sep 25, 2023
@gortiz gortiz mentioned this pull request Sep 27, 2023
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing Related to tests or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants