bretthoerner / dropwizard-scala

Scala helpers for Dropwizard.
86 stars 26 forks source link

Cross compiling. #1

Closed nbauernfeind closed 11 years ago

nbauernfeind commented 11 years ago

Any reasons you aren't cross compiling for all [popular] versions of scala? Jackson supports these versions of scala: 2.9.1, 2.9.2, 2.9.3, 2.10.0

bretthoerner commented 11 years ago

No reason other than laziness. I'll add the other versions now.

But Nathaniel, my buddy, my friend. Let me know when you're looking to move to greener pastures.

nbauernfeind commented 11 years ago

Well, you know, it's not just me =). Sometimes there are just things that are stuck on older versions (I think everything from Twitter is stuck on 2.9.2 for a little while yet, etc).

I did happen to realize I can upgrade straight to 2.10.0 for this particular project though.

My thoughts were that if jackson-module-scala is going to support them, then there is little reason not to keep cross compiling until they stop or something that's hard to maintain stops compiling. =)

nbauernfeind commented 11 years ago

While I have your attention, it looks like your release was not perfect. Somehow ScalaBundle came across targeted to a different jvm version. Do you mind re-releasing it? (perhaps 0.6.2-1 depending on the versioning scheme you were thinking of taking).

~/code/AgentSmith/tmp$ wget -q -O dropwizard-scala.jar http://search.maven.org/remotecontent?filepath=com/massrelevance/dropwizard-scala_2.10/0.6.2/dropwizard-scala_2.10-0.6.2.jar

~/code/AgentSmith/tmp$ jar xf dropwizard-scala.jar 

~/code/AgentSmith/tmp$ file com/yammer/dropwizard/ScalaService.class 
com/yammer/dropwizard/ScalaService.class: compiled Java class data, version 50.0 (Java 1.6)

~/code/AgentSmith/tmp$ file com/yammer/dropwizard/bundles/ScalaBundle.class 
com/yammer/dropwizard/bundles/ScalaBundle.class: compiled Java class data, version 51.0
bretthoerner commented 11 years ago

So there's a little pain in cross building because I use the latest specs2 which is only available for 2.10 apparently. I can cross built with different versions of specs but I think I have to convert over to a Project.scala format so I can def some shit. I'll try to do this soon.

nbauernfeind commented 11 years ago

I don't migrating all of the tests to Scalacheck. I'd also prefer for many of them to be like this: https://github.com/codahale/dropwizard/blob/master/dropwizard-core/src/test/java/com/yammer/dropwizard/jersey/tests/OptionalQueryParamInjectableProviderTest.java

I'd be happy to do that work, interested?

bretthoerner commented 11 years ago

Would Scalacheck help the problem?

I don't mind tests being refactored at all if you want to do it.

nbauernfeind commented 11 years ago

Scalacheck helps the cross-compilation problem. It should be a relatively trivial change. Jackson-module-scala uses it in nearly exactly the same way I'm seeing Spec2 being used in this repository. See this for an example.

The Guava-Optional test I showed you from Dropwizard is a test format I like because it goes through the trouble of being round-trip w.r.t. Jersey. It's a bit easier to follow when people ask "how do I use this and what does it give me?" and "are you sure this works?" That and we have ourselves covered across dropwizard/jersey changes that ultimately are incompatible with our Scala approach.

nbauernfeind commented 11 years ago

Sorry, I didn't mean scalacheck, I meant scalatest. Brain thinks one thing, fingers type something else. Sorry for the confusion.

bretthoerner commented 11 years ago

Sounds good to me, I've made you a collab on this and metrics-scala. Thanks.

nbauernfeind commented 11 years ago

Do you want to do code reads or do you prefer that I just push code changes?

bretthoerner commented 11 years ago

Either way, I'm not picky. I'm a bit heads down just using what we have now, but I can package & publish if you get cross compiling working via the test change.

nbauernfeind commented 11 years ago

Ok. I'll still have them go through as pull requests, but I'll manually merge them so the changes have some context. I'll post back when it should be good to go for a release.

nbauernfeind commented 11 years ago

OK. The repo should be good for a release. I've included a .travis.yml file to delegate build status to travis ci. You'll have to enable it to run on the main fork and update the Readme to use that repo for the status image. Please feel free to take a look at the code changes and suggest improvements.

I dropped the dependency on metrics-scala since they were not directly being used. You may need to update any project that you're using this in to also declare metrics-scala if you were only getting it as a transitive dependency.

Next my goals are to write some more round-trip like tests for the Jersey parameter parsing as well as get it to work for more than Strings (such as a Set[Int]). But, you should be able to release these changes as is completely backward compatible (except for the metrics-scala dependency).

bretthoerner commented 11 years ago

It looks good to me.

I'm trying to do a clean build before I attempt to publish, have to run into this before? ScalaBundle is still coming out at version 51 (note that I nuked my target/ directory first).

https://gist.github.com/bretthoerner/6d361d98475ffe354529

nbauernfeind commented 11 years ago

Good catch! Back in the day ScalaBundle was written in Java so that he could inspect (on the non-scala side of dropwizard) and yell at you if you seem to be using Scala but not the ScalaBundle. That has since disappeared, and as a result so has the need for that thing to be written in Java.

So I've pushed another pull request. This should not be an issue anymore. Please try again =), and thanks for double checking!

bretthoerner commented 11 years ago

Ah ha, makes sense. Sorry, I'm sort of new to the ecosystem.

I just published the crossbuilt release of 0.6.2-1, it should be on central mirrors within a few hours.

Thanks!