Closed awjones76 closed 4 years ago
Thanks for the Pull Request. I always very happy if someone extends the gradle xcode plugin. I have though the HockeyApp shutdown myself a few days ago :-). Because I don't use HockeyApp or App Center myself, it is not very likely that I implement this feature.
I have reviewed your pull requests and I have some comments. In the softwareproject I work on I try to keep 3rd party dependencies to a minimum.
In the plugin I have already the httpclient
added from the apache commons project. So is it necessary to add another http client. I also have created a HttpUpload
helper class that encapsulates the httpclient
itself. I would prefere if all the http requests are using the HttpUpload
class. If the okhttp
is a better choice than httpclient
than the HttpUpload
class should be modified and teh httpclient
removed as dependency.
Also Groovy has builtin JSON support. So I'm wondering if adding gson
dependency so much more functionally and/or comfort.
Another thing is unit tests. When you investigate the project you will see a lot of them. When I started this project (a long time ago) I havn't written lot of unit tests back then, because I never thought is project will get this big. That was a mistake. But this is another story ;-) Nowadays I do not write any production without any unit tests. I know that the HockeyApp Task itself has to few unit tests and also some other Tasks too. But when I touch old code like this, one thing I always do is to add unit tests. I would be great if you could add more unit tests for the new code, so that also the http upload itself is covered by unit tests.
If you like you can also remove the old HockeyApp code, otherwise I will do this myself in the near future.
Thank you for the feedback! I performed some refactoring based on your comments. I removed the HockeyApp plugin, replaced the Apache HTTP client with OkHttp, removed gson, and refactored the HttpPost class (renamed to HttpUtil). I still need to work on adding some more unit tests for the App Center code.
removing HockeyApp to this time seems to be a little bit early because if you have a lot of projects/customers you don't want to move all at the same time.
🤔 yesterday I already thought of pressing the "Merge pull request" button. Microsoft says that on 16. Nov the Hockeyapp services will shut down. My plan for now is that I want to updated some stuff for Xcode 11 compatibility (e.g. #416, and other stuff that will pop up). If this is done want to release this version, and then I want to merge this pull request. Then you have some more time to migrate, but also can use Xcode 11. As fallback you can always specify exactly which plugin version should be used. Is this fine with everybody?
That sounds like a good plan. Makes sense to me!
Thanks again!
Using a ,
separated string for destination
like appcenter.destination=App Center Testgroup,Collaborators
does not work. It only works with one destination.
> Task :appcenter FAILED
FAILURE: Build failed with an exception.
* What went wrong:
Execution failed for task ':appcenter'.
> Http request failed: 400 Bad Request: {"mandatory_update":false,"release_notes":"This build was uploaded using the gradle xcodeplugin","code":"bad_request","message":"Distribution Group with id undefined does not exist."}
Using a
,
separated string fordestination
likeappcenter.destination=App Center Testgroup,Collaborators
does not work. It only works with one destination.> Task :appcenter FAILED FAILURE: Build failed with an exception. * What went wrong: Execution failed for task ':appcenter'. > Http request failed: 400 Bad Request: {"mandatory_update":false,"release_notes":"This build was uploaded using the gradle xcodeplugin","code":"bad_request","message":"Distribution Group with id undefined does not exist."}
Using a
,
separated string fordestination
likeappcenter.destination=App Center Testgroup,Collaborators
does not work. It only works with one destination.> Task :appcenter FAILED FAILURE: Build failed with an exception. * What went wrong: Execution failed for task ':appcenter'. > Http request failed: 400 Bad Request: {"mandatory_update":false,"release_notes":"This build was uploaded using the gradle xcodeplugin","code":"bad_request","message":"Distribution Group with id undefined does not exist."}
I made an update so you can now provide a list of distribution groups in the form of:
destination = ["Collaborators", "QA Testers"]
Added Microsoft App Center integration because HockeyApp will be transitioning to App Center on November 16, 2019.