Skip to content
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

[MSHARED-826] Require Java 7 #7

Merged
merged 4 commits into from
Jun 11, 2019
Merged

[MSHARED-826] Require Java 7 #7

merged 4 commits into from
Jun 11, 2019

Conversation

mthmulders
Copy link
Contributor

As suggested by @rfscholte in MSHARED-826: updated to maven-parent:33 and set property javaVersion to 7.

@rfscholte
Copy link
Contributor

The OS related tests need to be changed, see https://builds.apache.org/job/maven-box/job/maven-shared-utils/job/MSHARED-826/test_results_analyzer/
With Java 12 you cannot change the System properties anymore that were provided by Java at startup.

@Tibor17
Copy link
Contributor

Tibor17 commented Jun 5, 2019

@rfscholte
The Java 12 has a new API to override system properties at runtime?

@rfscholte
Copy link
Contributor

@Tibor17 No, there's just a set of properties that are marked as static properties: they can only be set at startup and cannot be changed afterwards. https://github.com/AdoptOpenJDK/openjdk-jdk11/blob/master/src/java.base/share/classes/jdk/internal/util/StaticProperty.java shows a list.
However, it doesn't mention the os properties... might need some extra analysis.

This way, we can first change the system props
and then interact with the SUT. This way, changing
the static class fields is no longer needed.
@Tibor17
Copy link
Contributor

Tibor17 commented Jun 6, 2019

@mthmulders
One thing is to set higher Java 1.7 and another thing is to use Java 1.7 features in Java code.
You know the features of Java 1.7.
Try to open another PR where you complete this improvement in code; otherwise bytecode version does not have any value for nobody. Thx for your work!

@mthmulders
Copy link
Contributor Author

Thanks for the suggestion. I deliberately didn't want to mix the two (requiring Java 1.7 and using its new features). Java 7 is required for MSHARED-822 / #6, so we'll soon make use of at least some of the new features.

@rfscholte rfscholte merged commit ba67166 into apache:master Jun 11, 2019
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