codingwell / scala-guice

Scala extensions for Google Guice
Apache License 2.0
341 stars 44 forks source link

Common changes for Scala[Option, Map, Multi]Binders. #32

Closed nbauernfeind closed 10 years ago

nbauernfeind commented 10 years ago

I ended up writing some code for ScalaMapBinder as well and ended up refactoring quite a few things and coming up with something that's slightly cleaner for ScalaOptionBinder and ScalaMultibinder.

This pull request is the common stuff across all three of them; so you'll probably want to look at it first as you'll see these changes in each of those pull requests too.

tsuckow commented 10 years ago

The last commit should really be applied after the option/map/multi changes. I'll rebase it myself.

nbauernfeind commented 10 years ago

I agree. Only reason I hadn't is because github doesn't handle dependent pull requests very well. I've pulled it out of the PR; I'll create a new one as soon as the other changes are merged.

nbauernfeind commented 10 years ago

I hadn't realized you pushed them manually to the develop branch already. I noticed github, for some reason, hadn't noticed you already merged those commits from those pull requests. So I 'touched' the branches to get github to ignore those commits. This PR now contains only the readme as well.

nbauernfeind commented 10 years ago

Anything else cross your mind @tsuckow?

tsuckow commented 10 years ago

So far, nothing else is on my mind. It just takes me a while to read through it and make sure I am satisfied with how it works. It doesn't help that I have been on two work trips in the last week. I also had to buy a new car before my old one left me completely stranded. I have a family obligation this weekend, but hopefully can continue sunday evening.

Thanks for all your hard work Nate! And happy belated birthday!

tsuckow commented 10 years ago

Alright. If you could make sure I haven't screwed up your changes somehow, that would be awesome. If you have time, I would like to resolve #34 before rolling the 4.0.0-beta5 release. If not I will hopefully get to it in a few days. Then comes remembering how to do maven central.