Fix #23, fix #33: close uploadStub when closing PhotosLibraryClient #40
Conversation
...raryapi/src/main/java/com/google/photos/library/v1/internal/InternalPhotosLibraryClient.java
Show resolved
Hide resolved
jfschmakeit
left a comment
There was a problem hiding this comment.
Thanks for this pull request, this looks like it addresses the issues.
There are a few complications around changing a class in the internal package (and in particular change the signature of a method there), so we need to consider this carefully. I'm following up on this and will keep you posted.
photoslibraryapi/src/main/java/com/google/photos/library/v1/PhotosLibraryClient.java
Show resolved
Hide resolved
|
I have just filed a request with the gapic-generator-java team here: googleapis/sdk-platform-java#650 In the meantime we should be able to submit this and roll this into a new release. Please stay tuned. |
jfschmakeit
left a comment
There was a problem hiding this comment.
Unfortunately this is currently not passing the tests, please make sure to format the changed files again:
The following files are not formatted properly:
photoslibraryapi/src/main/java/com/google/photos/library/v1/PhotosLibraryClient.java
:photoslibraryapi:verifyGoogleJavaFormat (Thread[Execution worker for ':',5,main]) completed. Took 0.017 secs.FAILURE: Build failed with an exception.
- What went wrong:
Execution failed for task ':photoslibraryapi:verifyGoogleJavaFormat'.Problems: formatting style violations
Run the gradle task test(or the :photoslibraryapi:verifyGoogleJavaFormat task) to verify your change.
You can automatically format the files using the googleJavaFormat gradle task.
Once you have addressed this we can merge it and make it a new release.
Thanks!
|
Apologies @jfschmakeit, I thought I changed my column width...I should have checked anyway. Should be good now. |
|
Great - thanks for bearing with me through the review! We are all good to merge this into a new release. (Unfortunately the gapic team isn't going to take our proposal on board, so we'll have to do some pre-processing on our end. We can merge this manual change in for now and update our pipeline to include this in the future.) |
No description provided.