Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support latest Retrofit 2 v2.0.0-beta4 #2149

Merged
merged 6 commits into from
Feb 29, 2016
Merged

support latest Retrofit 2 v2.0.0-beta4 #2149

merged 6 commits into from
Feb 29, 2016

Conversation

dhontecillas
Copy link
Contributor

Retrofit v2.0.0-beta4 introduces some package changes from beta3, that should be reflected in moustache templates.

@wing328
Copy link
Contributor

wing328 commented Feb 17, 2016

@dhontecillas thanks for the PR. Please update retrofit2 petstore sample so that CI can verify the change.

Ref: https://github.com/swagger-api/swagger-codegen/blob/master/CONTRIBUTING.md#testing

@wing328 wing328 added this to the v2.1.6 milestone Feb 17, 2016
@DerPate
Copy link

DerPate commented Feb 17, 2016

Maybe you also push the okhttp version to okhttp3 because the beta4 for retrofit pushes this also to the same version and at least introduced some bugs at my instaltion

@wing328
Copy link
Contributor

wing328 commented Feb 18, 2016

Btw, one of your commits (in the Commits tab) is not linked to your account, which means that commit won't count as your contribution in https://github.com/swagger-api/swagger-codegen/graphs/contributors

@cbornet
Copy link
Contributor

cbornet commented Feb 18, 2016

I think the okhttp3 dependency should be removed since it is already included in retrofit2

@wing328
Copy link
Contributor

wing328 commented Feb 22, 2016

@dhontecillas may I know if you're updating the PR with the feedback from @cbornet and @DerPate? Let us know if you need any help.

@dhontecillas
Copy link
Contributor Author

I'm sorry, I'm crunching for a release that I have the Feb 24th, so I don't expect to fix the pull request until the end of this week. If that is no problem, I will update it.

@wing328
Copy link
Contributor

wing328 commented Feb 22, 2016

Thanks for the update. Please take your time :)

@dhontecillas
Copy link
Contributor Author

I've updated the tests for retrofit2 library, as suggested in the comments. Can you check everything is ok ?

@wing328
Copy link
Contributor

wing328 commented Feb 25, 2016

@dhontecillas I did some tests and found some minor issues:

  1. When I ran gradle check under samples/client/petstore/java/retrofit2, I got the following error:
/private/var/tmp/pr/dhontecillas/swagger-codegen/samples/client/petstore/java/retrofit2/src/main/java/io/swagger/client/ApiClient.java:15: error: package retrofit2.converter.gson does not exist
import retrofit2.converter.gson.GsonConverterFactory;
                               ^
/private/var/tmp/pr/dhontecillas/swagger-codegen/samples/client/petstore/java/retrofit2/src/main/java/io/swagger/client/ApiClient.java:324: error: cannot find symbol
      private final GsonConverterFactory gsonConverterFactory;
                    ^
  symbol:   class GsonConverterFactory
  location: class GsonCustomConverterFactory
/private/var/tmp/pr/dhontecillas/swagger-codegen/samples/client/petstore/java/retrofit2/src/main/java/io/swagger/client/ApiClient.java:329: error: cannot find symbol
        this.gsonConverterFactory = GsonConverterFactory.create(gson);
                                    ^

I believe you need to update build.gradle (mustache) as well.

  1. I ran ./bin/java-petstore-retrofit2rx.sh and got the following error when running mvn test under samples/client/petstore/java/retrofit2rx
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:2.3.2:compile (default-compile) on project swagger-petstore-retrofit2-rx: Compilation failure: Compilation failure:
[ERROR] /private/var/tmp/pr/dhontecillas/swagger-codegen/samples/client/petstore/java/retrofit2rx/src/main/java/io/swagger/client/ApiClient.java:[15,31] error: package retrofit2.converter.gson does not exist
[ERROR] /private/var/tmp/pr/dhontecillas/swagger-codegen/samples/client/petstore/java/retrofit2rx/src/main/java/io/swagger/client/ApiClient.java:[324,17] error: cannot find symbol
[ERROR] class GsonCustomConverterFactory
[ERROR] /private/var/tmp/pr/dhontecillas/swagger-codegen/samples/client/petstore/java/retrofit2rx/src/main/java/io/swagger/client/ApiClient.java:[329,33] error: cannot find symbol
[ERROR] class GsonCustomConverterFactory
[ERROR] /private/var/tmp/pr/dhontecillas/swagger-codegen/samples/client/petstore/java/retrofit2rx/src/main/java/io/swagger/client/ApiClient.java:[340,4] error: method does not override or implement a method from a supertype

@dhontecillas
Copy link
Contributor Author

I fixed the templates but I still have to check the gradle build.

@dhontecillas
Copy link
Contributor Author

I think that now it should be ok and ready to merge.

@wing328
Copy link
Contributor

wing328 commented Feb 29, 2016

@dhontecillas tested again and no longer encountered those issues. Thanks again for the PR.

wing328 added a commit that referenced this pull request Feb 29, 2016
support latest Retrofit 2 v2.0.0-beta4
@wing328 wing328 merged commit d665903 into swagger-api:master Feb 29, 2016
@dhontecillas
Copy link
Contributor Author

You are welcome. Thanks for the patience with the not so well done PR :)

@wing328
Copy link
Contributor

wing328 commented Feb 29, 2016

Given that it's your first PR to swagger-codegen, it definitely looks good. I hope by now you feel more comfortable making changes to swagger-codegen.

If you're interested in making more enhancements to the Java API client, please refer to https://github.com/swagger-api/swagger-codegen/issues?q=is%3Aopen+label%3A%22Need+community+contribution%22+label%3A%22Client%3A+Java%22

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants