Skip to content

[CALCITE-2594] Ensure forbiddenapis and maven-compiler use the correct JDK version#74

Merged
asfgit merged 1 commit intoapache:masterfrom
risdenk:CALCITE-2594
Sep 25, 2018
Merged

[CALCITE-2594] Ensure forbiddenapis and maven-compiler use the correct JDK version#74
asfgit merged 1 commit intoapache:masterfrom
risdenk:CALCITE-2594

Conversation

@risdenk
Copy link
Contributor

@risdenk risdenk commented Sep 25, 2018

No description provided.

pom.xml Outdated
<configuration>
<source>8</source>
<target>8</target>
<source>${maven.compiler.source}</source>
Copy link
Member

Choose a reason for hiding this comment

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

I think these are superfluous when you have maven.compiler.source and maven.compiler.target set as you 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.

Yea I think so too. I just wanted to be sure we set the same version in one spot. Afraid someone comes along later and updates the 8 to a 9/10/11 and doesn't change the maven.compiler.source/maven.compiler.target properties.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe drop them here, and leave a comment not to re-add?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup that works. I'll make that change.

…t JDK version

Close apache#74

Signed-off-by: Kevin Risden <krisden@apache.org>
@risdenk
Copy link
Contributor Author

risdenk commented Sep 25, 2018

Thanks for the review @joshelser!

@asfgit asfgit merged commit 8db4bc1 into apache:master Sep 25, 2018
@risdenk risdenk deleted the CALCITE-2594 branch September 25, 2018 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants