Skip to content
This repository has been archived by the owner on Jun 19, 2024. It is now read-only.

Java11 #1437

Merged
merged 3 commits into from
Nov 16, 2018
Merged

Java11 #1437

merged 3 commits into from
Nov 16, 2018

Conversation

ffang
Copy link
Contributor

@ffang ffang commented Nov 16, 2018

No description provided.

@ffang
Copy link
Contributor Author

ffang commented Nov 16, 2018

With this PR, "mvn clean install" are successful both on JDK8 and JDK11.

@codecov
Copy link

codecov bot commented Nov 16, 2018

Codecov Report

Merging #1437 into master will increase coverage by <.01%.
The diff coverage is 50%.

@@             Coverage Diff              @@
##             master    #1437      +/-   ##
============================================
+ Coverage     34.87%   34.87%   +<.01%     
  Complexity     1046     1046              
============================================
  Files           171      171              
  Lines          9431     9433       +2     
  Branches       1611     1612       +1     
============================================
+ Hits           3289     3290       +1     
  Misses         5783     5783              
- Partials        359      360       +1

@@ -66,6 +66,10 @@ public String getLabel() {
* Returns true if the given maven properties indicate running in OpenShift platform mode
*/
public static boolean isOpenShiftMode(Properties properties) {
return Objects.equal(openshift.toString(), properties.getProperty(FABRIC8_EFFECTIVE_PLATFORM_MODE, ""));
if (properties == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I will use a ternary in this case just for make code clean, but just my opinion of course.

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, good point!

@@ -71,6 +69,9 @@ public void javaOptions() throws IOException, MojoExecutionException {
assertEquals(1, configs.size());
Map<String, String> env = configs.get(0).getBuildConfiguration().getEnv();
assertNull(env.get("JAVA_OPTIONS"));
} catch (Exception ex) {
ex.printStackTrace();
Copy link
Contributor

Choose a reason for hiding this comment

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

can you make the test make error by rethrowing the exception?
Also if there is an exception the test will fail automatically and print the stack trace.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, that just for my debug and surely should be removed.
Another commit is coming soon...

@lordofthejars lordofthejars merged commit a5f3343 into fabric8io:master Nov 16, 2018
@lordofthejars
Copy link
Contributor

Thank you very much for the contribution.

@rohanKanojia rohanKanojia mentioned this pull request Nov 23, 2018
lordofthejars pushed a commit to lordofthejars/fabric8-maven-plugin that referenced this pull request Jan 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants