search

stephanenicolas / robospice

Repo of the Open Source Android library : RoboSpice. RoboSpice is a modular android library that makes writing asynchronous long running tasks easy. It is specialized in network requests, supports caching and offers REST requests out-of-the box using extension modules.
Apache License 2.0
2.95k stars 545 forks source link

Add a robospice-retrofit2 module to support a new Retrofit of version 2.0 and up #455

Closed mykolaj closed 8 years ago

mykolaj commented 8 years ago

Add a robospice-retrofit2 module to support a new Retrofit of version 2.0 and up

rciovati commented 8 years ago

I'd continue here the discussion about naming that we started in #454. @stephanenicolas What do you think about the naming thing?

mykolaj commented 8 years ago

At the beginning i've started to work on this addon inside the existing robospice-retrofit module. But it soon it became clear to me that the new functionality conflicts with the old one and the both can not co-exist inside the same module, or will make implementation of the module quite messy. So i've created this new robospice-retrofit2 extension.
Also there were talks about a new Retrofit version out there for quite a time - since last summer i guess. And it is often referenced as "Retrofit two point o". So it was logically attractive to call the new extension as "robospice-retrofit2". So, everyone would make parallels like "retrofit two point o -> robospice-retrofit2".
So i think a new name should be something that clearly distinguish "old retrofit" from "new retrofit" at least.

rciovati commented 8 years ago

I absolutely agree about having another module. I'm just concerned about having retrofit2 in the package name and in the class names :)

mykolaj commented 8 years ago

OK. What if i remove every "2" suffix from every class name so the class names will match the names from the robospice-retrofit module. This way for example RetrofitSpiceService2 will remain RetrofitSpiceService, RetrofitObjectPersister2 will remain RetrofitObjectPersister, and so on. Everything like in previous version.
Instead i just move all classes in new packages with names like com.octo.android.robospice.persistence.retrofit.second, or com.octo.android.robospice.persistence.retrofit.two, or com.octo.android.robospice.persistence.retrofit.v2. Again, everything like in previous version, but with extra package directory added. This way a migration from the previous extension to the new one will be as simple as just change imports, and implement one extra method for every SpiceService implementation.
But about module name itself... i don't know, maybe robospice-retrofit-v2?
Also there are a couple of ridiculous choices: robospice-retrofit-next, robospice-retrofit-second, robospice-retrofit-new, robospice-retrofit-nextgen :)

rciovati commented 8 years ago

IHMO it would be fine to have the package name like com.octo.android.robospice.persistence.retrofit2 so that it would match the Retrofit 2 package. Same for the module name, robospice-retrofit2 looks good to me, but I'd wait for someone else to approve :)

mykolaj commented 8 years ago

Alright. As for now, i've removed '2' from class names.

mykolaj commented 8 years ago

I'm closing this pull request for now because there is a plenty of work to do about this new feature. Will create a new pull request when the work is done.

mykolaj commented 8 years ago

459 as i said earlier