Skip to content

Conversation

@dhermes
Copy link
Contributor

@dhermes dhermes commented Oct 12, 2016

NOTE: Has #2531 as diffbase.

In doing this, I realized it's a bit tedious to have to write two classes (page and iterator) for each method. It may be more user-friendly to collapse back into one class. I think I could do it mostly by just making anything that is accessed via self.page.foo as self.page_foo.

@dhermes dhermes added the api: dns Issues related to the Cloud DNS API. label Oct 12, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Oct 12, 2016
@dhermes
Copy link
Contributor Author

dhermes commented Oct 17, 2016

@daspecster @tseaver @jonparrott PTAL.

There will be about 5 more PRs like this for each package which does page token list_foo().

Copy link
Contributor

@theacodes theacodes left a comment

Choose a reason for hiding this comment

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

Looks pretty straightforward to me.

@dhermes
Copy link
Contributor Author

dhermes commented Oct 17, 2016

TRAVIS!

@theacodes
Copy link
Contributor

TRAVIS!

No travis for you, I used up all of his cycles in google-auth. :P

@tseaver
Copy link
Contributor

tseaver commented Oct 17, 2016

I don't really care for the inheritance-based iterator approach: would it be better to tackle that first in #2531#2548? Or merge this and then maybe refactor, killing of the subclasses?

@dhermes
Copy link
Contributor Author

dhermes commented Oct 18, 2016

@tseaver Can you weigh in on #2558 ASAP?

@dhermes dhermes closed this Oct 18, 2016
@dhermes dhermes deleted the dns-iterators branch October 18, 2016 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: dns Issues related to the Cloud DNS API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants