netceteragroup / valdr-bean-validation

Java Bean Validation plugin
MIT License
35 stars 21 forks source link

Added Java 8 compatibility. #23

Closed robert-iddink closed 9 years ago

robert-iddink commented 9 years ago

Some minor change to get it working with Java 8.

buildhive commented 9 years ago

Netcetera » valdr-bean-validation #85 FAILURE Looks like there's a problem with this pull request (what's this?)

robert-iddink commented 9 years ago

The build failed, because it ran with JDK7.

robert-iddink commented 9 years ago

@marcelstoer Can you have a look at this PR? Thanks!

buildhive commented 9 years ago

Netcetera » valdr-bean-validation #87 FAILURE Looks like there's a problem with this pull request (what's this?)

buildhive commented 9 years ago

Netcetera » valdr-bean-validation #88 FAILURE Looks like there's a problem with this pull request (what's this?)

buildhive commented 9 years ago

Netcetera » valdr-bean-validation #89 FAILURE Looks like there's a problem with this pull request (what's this?)

marcelstoer commented 9 years ago

Sorry, I totally forgot about this PR. Thanks for the reminder. I can (and should) add Java 8 compatibility but there are three issues with this PR:

robert-iddink commented 9 years ago

OK, I'll try to fix those first. Thanks for your quick reply!

coveralls commented 9 years ago

Coverage Status

Coverage increased (+0.19%) to 90.6% when pulling b3703707b60244bed65964c96051dc8dcc05c8f7 on Arzie:master into 326986475e596401eb24f512e5c9ac8c2e58b91b on netceteragroup:master.

buildhive commented 9 years ago

Netcetera » valdr-bean-validation #90 UNSTABLE Looks like there's a problem with this pull request (what's this?)

robert-iddink commented 9 years ago

There we go ;-). Did I fix the whitespace issue (I'm not sure where the problem was)?

buildhive commented 9 years ago

Netcetera » valdr-bean-validation #91 SUCCESS This pull request looks good (what's this?)

robert-iddink commented 9 years ago

OK, so either the buildhive build passes (which runs on Java 1.7), or the Travis build passes (which runs on 1.8). I'll apply the test fixes once more, so the build will pass on Java 1.8 (which we're targeting anyway).

buildhive commented 9 years ago

Netcetera » valdr-bean-validation #92 UNSTABLE Looks like there's a problem with this pull request (what's this?)

buildhive commented 9 years ago

Netcetera » valdr-bean-validation #93 UNSTABLE Looks like there's a problem with this pull request (what's this?)

coveralls commented 9 years ago

Coverage Status

Coverage increased (+0.19%) to 90.6% when pulling c2ec06e449926d5c1d4ddcdd3ccc94b6589d7914 on Arzie:master into 326986475e596401eb24f512e5c9ac8c2e58b91b on netceteragroup:master.

robert-iddink commented 9 years ago

@marcelstoer I think I fixed the issues, can you have another look?

The tests still give some trouble, maybe they should be made more stable somehow? They're dependent on the exact form of the JSON, which seems to change from build to build.

marcelstoer commented 9 years ago

I'm taking a look now but you certainly can't run a Maven build targeting Java 8 with a Java 7 compiler. For Travis this information is in .travis.yml for BuildHive and drone.io it's in the build job configuration.

robert-iddink commented 9 years ago

Well, the pom.xml now targets 1.7 again, so that should work fine. I updated the Travis config file to make it run on JDK 8, but I can't change the BuildHive config.

marcelstoer commented 9 years ago

@Arzie Given that

I was wondering what you really were trying to accomplish. Is reflections 0.9.9 required if you want to run on a JVM 8? I didn't find any such reference in their documentation.

robert-iddink commented 9 years ago

The main change is indeed the version of the reflections dependency. If I remember correctly this was the issue we had with it: https://code.google.com/p/reflections/issues/detail?id=169

So if you're trying to analyse Java 8 sources with valdr-bean-validation, you're not getting the results you want.

marcelstoer commented 9 years ago

Ahh, thanks for the link to that issue. I only checked their issues on GitHub. And thanks for pointing out the problems with that unit test. I played around with different JDKs and can confirm that the order of the JSON attributes changes. I'll fix that.

robert-iddink commented 9 years ago

Thanks Marcel!

marcelstoer commented 9 years ago

I just released 1.1.1, should show up in Maven Central shortly.

robert-iddink commented 9 years ago

Cool, I'll start using it right away ;-).