search

edulify / play-geolocation-module.edulify.com

This is a geolocation plugin for Playframework
https://edulify.github.io/play-geolocation-module.edulify.com/
Apache License 2.0
22 stars 11 forks source link

Play-2.4 version #8

Closed sovaalexandr closed 9 years ago

sovaalexandr commented 9 years ago

DI - oriented and Play! Framework 2.4 ready version of Geo-location module

megazord commented 9 years ago

@sovaalexandr thank you so much for this PR. I don't have a problem with it being created to master, since there are no differences between master and play-2.4.0 branches.

Please, consider my comments more as questions. I really appreciate your help and time to move this module forward.

sovaalexandr commented 9 years ago

@megazord thank you having time to review my code. I'll make changes to this branch ASAP

sovaalexandr commented 9 years ago

@megazord I had just finished my changes on returning back plugin and corrected documentation But right now provider code seems uglier to me. For instance I'd prefer to use constructor injection rather than setter-injection and so on. But in considerations of backwards-compatibility I tried to make plugin and provider instantiation as flexible as possible. I also removed GeolocationFactory and processed each your feedback. But I still want to make code of this component more strict.

megazord commented 9 years ago

@sovaalexandr this PR was merged manually into branch play-2.4.0. There are some points that I want to change before doing a release (or maybe a milestone release). If you decide to submit more pull requests, please submit them against branch play-2.4.0.

Thank you so much!

sovaalexandr commented 9 years ago

@megazord Welcome) You can also close issue #7