-
Notifications
You must be signed in to change notification settings - Fork 13.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FLINK-33159] [build] use variables in pom file for Java and Maven versions #23469
base: master
Are you sure you want to change the base?
Conversation
thanks @MartijnVisser . I am looking forward to your review @zentol :-) . If we are ok with this, please could you assign me the issue and pr. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! Added 2 comments. In general I believe if we change these props it would worth to explore the whole codebase, so we do not leave any loose ends.
After a quick check I found that the POM of flink-walkthrough-datastream-java
archetype defines a raw 1.8 as target.java.version
, but it should probably inherit that value from the root POM variable as well with the @target.java.version@
idiom.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added 1 additional comment, the rest seems right.
...alkthroughs/flink-walkthrough-datastream-java/src/main/resources/archetype-resources/pom.xml
Show resolved
Hide resolved
@ferenc-csaky thanks I have made the requested change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the improvements, LGTM!
@zentol are you ok to merge this please? |
Signed-off-by: David Radley <david_radley@uk.ibm.com>
54498b6
to
3ca812e
Compare
This PR is being marked as stale since it has not had any activity in the last 180 days. If you are having difficulty finding a reviewer, please reach out to the If this PR is no longer valid or desired, please feel free to close it. |
What is the purpose of the change
Use Maven variables for all cases when referring to Java and Maven versions in the pom file
Brief change log
I have introduced 2 Maven variables for the 2 versions that are referenced in the build.
I have ensured all references to Java 1.8 use the java variable
Verifying this change
Please make sure both new and modified tests in this PR follows the conventions defined in our code quality guide: https://flink.apache.org/contributing/code-style-and-quality-common.html#testing
This change is a trivial rework / code cleanup without any test coverage.
Does this pull request potentially affect one of the following parts:
-no
Documentation