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

fix(pubsublite): fixed return value of AdminClient.TopicSubscriptions #3220

Merged
merged 2 commits into from Nov 25, 2020

Conversation

tmdiep
Copy link
Contributor

@tmdiep tmdiep commented Nov 17, 2020

It should only return an iterator.
Also added unit tests.

Also fixed the return value of TopicSubscriptions.
@google-cla google-cla bot added the cla: yes This human has signed the Contributor License Agreement. label Nov 17, 2020
@product-auto-label product-auto-label bot added the api: pubsublite Issues related to the Pub/Sub Lite API. label Nov 17, 2020
@tmdiep tmdiep marked this pull request as ready for review November 18, 2020 05:04
@tmdiep tmdiep requested a review from a team as a code owner November 18, 2020 05:04
@tmdiep tmdiep requested a review from hongalex November 18, 2020 05:04
@tmdiep tmdiep changed the title feat(pubsublite): Admin client unit tests fix(pubsublite): fixed return value of AdminClient.TopicSubscriptions Nov 24, 2020
@@ -0,0 +1,50 @@
// Copyright 2020 Google LLC
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be a duplicate of internal/wire/main_test. Is there a reason why it's being added here too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, this was unfortunate. TestMain only applies to the package where it resides. I will have to copy to a third package, so might move the bulk of it to a common util (probably in a separate PR, since this one is already approved) shared by all the main_test.go clones.

@tmdiep
Copy link
Contributor Author

tmdiep commented Nov 25, 2020

Thanks Alex. Will do the refactoring for main_test.go in a separate PR. Also wanted to point out that the change to TopicSubscriptions is technically a breaking change, but since this module is still in early development, I didn't mark it as such. Wasn't sure if the auto release tool would bump up the major revision number.

@tmdiep tmdiep merged commit f37f118 into googleapis:master Nov 25, 2020
@tmdiep tmdiep deleted the admin_tests branch November 25, 2020 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsublite Issues related to the Pub/Sub Lite API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants