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 label Nov 17, 2020
@tmdiep tmdiep marked this pull request as ready for review Nov 18, 2020
@tmdiep tmdiep requested a review from as a code owner Nov 18, 2020
@tmdiep tmdiep requested a review from hongalex Nov 18, 2020
@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

@hongalex hongalex Nov 25, 2020

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

@tmdiep tmdiep Nov 25, 2020

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 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
3 checks passed
@tmdiep tmdiep deleted the admin_tests branch Nov 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants