Skip to content

Conversation

@CLOVIS-AI
Copy link

Adding getters using Optionals to be more "up-to-date" with Java 8 (see issue #94 for the discussion)

<source>1.5</source>
<target>1.5</target>
<source>1.8</source>
<target>1.8</target>
Copy link
Author

Choose a reason for hiding this comment

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

These two lines are needed to solve the problem found here on the discussion of issue #94

Copy link
Author

Choose a reason for hiding this comment

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

This caused more problems than it resolved, so it was reverted.

@CLOVIS-AI CLOVIS-AI changed the title Optionals [WIP] Optionals Apr 16, 2018
@CLOVIS-AI
Copy link
Author

There is currently a problem with org.codehaus.mojo.signature:java15:1.0 that dislikes every reference to any Optional object; stacktrace:

JsonObject.java:595: Undefined reference: java.util.Optional java.util.Optional.ofNullable(Object)

A line alike this one appears for each use of Optional, OptionalInt...

@sroughley
Copy link

I guess this is because optionals were new in Java 1.8?

@CLOVIS-AI
Copy link
Author

CLOVIS-AI commented Apr 16, 2018

That's probably why, but Travis is not having it, probably because of some dependency. I'm not knowledgeable in any way in the Travis ecosystem, so I don't think I will be able to solve that one, sadly... I think someone better at it might need to intervene

Otherwise, I think the work is fully done, all the getters in JsonObject were done and I didn't see any other that would need Optionals at this point (it wouldn't make sense for JsonArray#get).

@CLOVIS-AI CLOVIS-AI changed the title [WIP] Optionals Optionals Apr 16, 2018
@nbartels
Copy link
Contributor

nbartels commented Dec 8, 2018

The problem is in the pom.xml.

There is a build plugin called animal-sniffer-maven-plugin. In this plugin java 1.5 is checked. I think (but without trying it) you have to make this change:

<plugin>
   <groupId>org.codehaus.mojo</groupId>
   <artifactId>animal-sniffer-maven-plugin</artifactId>
   <version>1.13</version>
   <configuration>
     <signature>
      <groupId>org.codehaus.mojo.signature</groupId>
       <artifactId>java18</artifactId>
       <version>1.0</version>
     </signature>
   </configuration>
   <executions>
     <execution>
       <id>ensure-java-1.8-class-library</id>
       <phase>test</phase>
       <goals>
         <goal>check</goal>
       </goals>
     </execution>
   </executions>
</plugin>

Additionally the maven-bundle-plugin plugin needs to be changed.
The line <Bundle-RequiredExecutionEnvironment>J2SE-1.5</Bundle-RequiredExecutionEnvironment>should be changed to 1.8. The value seems to be JavaSE-1.8.

With these changes the optionals should work. If you like you can try this. Otherwise I can try to put them on top of your changes as soon as I get some time ;)

@CLOVIS-AI
Copy link
Author

I'm really busy right now, at most I'll be able to do it in ~week.

Also, it would be nice to add @NotNull and @nullable annotations to improve Kotlin compatibility: a short list of compatible extensions is found here and the full list can be found here (in the source code).

@nbartels
Copy link
Contributor

nbartels commented Dec 9, 2018

Additionally some null checks, in the Json class for example, can be replaced by Objects.requireNonNull.

And I think as soon as the source level is switched to 8 more cool java 8 features can be used, that make the code even better.

CLOVIS-AI added a commit to CLOVIS-AI/minimal-json that referenced this pull request Dec 9, 2018
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