Skip to content

[JENKINS-54842] Define compiler configuration with Maven properties where possible and use Java 11 built-in functionality instead of Animal Sniffer#523

Merged
basil merged 5 commits into
jenkinsci:masterfrom
basil:properties
Mar 31, 2022
Merged

Conversation

@basil

@basil basil commented Mar 30, 2022

Copy link
Copy Markdown
Member

This change should be functionally equivalent to the existing code, and by itself it doesn't buy us anything, but in concert with jenkinsci/maven-hpi-plugin#323, this allows us to require Java 11 in the future without an awkward flag day.

@basil basil added the internal label Mar 30, 2022
@basil
basil requested a review from jglick March 30, 2022 21:33
Comment thread pom.xml
<artifactId>maven-javadoc-plugin</artifactId>
<version>3.3.2</version>
<configuration>
<source>8</source>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The default value is ${maven.compiler.source}, which we are setting above.

Comment thread pom.xml
<properties>
<maven.compiler.release>8</maven.compiler.release>
<maven.compiler.testRelease>8</maven.compiler.testRelease>
<animal.sniffer.skip>true</animal.sniffer.skip>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Technically a change in behavior, but release serves the same purpose as Animal Sniffer.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Well, almost. With A.S. you can use @IgnoreJRERequirements on a particular class/method. I do not recommend it (better to use reflection) and would not miss that capability being dropped.

@basil basil changed the title Define compiler configuration with Maven properties where possible [JENKINS-54842] Define compiler configuration with Maven properties where possible Mar 31, 2022
@basil basil changed the title [JENKINS-54842] Define compiler configuration with Maven properties where possible [JENKINS-54842] Define compiler configuration with Maven properties where possible and use Java 11 built-in functionality instead of Animal Sniffer Mar 31, 2022

@jglick jglick left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

General style is to prefer <plugin> / <configuration> to setting <properties>, so this is slightly undesirable on its own, but of course the main consideration is whether or not we do jenkinsci/maven-hpi-plugin#323.

Comment thread pom.xml
<properties>
<maven.compiler.release>8</maven.compiler.release>
<maven.compiler.testRelease>8</maven.compiler.testRelease>
<animal.sniffer.skip>true</animal.sniffer.skip>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Well, almost. With A.S. you can use @IgnoreJRERequirements on a particular class/method. I do not recommend it (better to use reflection) and would not miss that capability being dropped.

@basil

basil commented Mar 31, 2022

Copy link
Copy Markdown
Member Author

Is there a request for change on this PR? If not, why was this PR not approved?

@jglick jglick left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes approved conditionally on jenkinsci/maven-hpi-plugin#323 being merged, sorry for being unclear.

@basil
basil merged commit 943a57b into jenkinsci:master Mar 31, 2022
@basil
basil deleted the properties branch March 31, 2022 17:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants