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

Implement ListOrganizations #101

Merged
merged 1 commit into from Jan 16, 2017
Merged

Implement ListOrganizations #101

merged 1 commit into from Jan 16, 2017

Conversation

gliptak
Copy link
Contributor

@gliptak gliptak commented Jan 8, 2017

#35

@jamesog @elithrar Please review and guide on pagination.

One thought would be to rename ResultInfo to Paginator and add a second method signature taking in Paginator. Or variadic args as per #4 ?

Thanks

@gliptak
Copy link
Contributor Author

gliptak commented Jan 13, 2017

Could you point out the preferred approach? Thanks

@jamesog
Copy link
Contributor

jamesog commented Jan 13, 2017

Hi @gliptak - thanks for this!

Pagination we've discussed a bit in #6 and #78. It's going to be a breaking change to implement it so it's probably worth doing everything at once. I've got a WIP I'm testing at the moment but needs a bit more work before I submit it.

For now I think this PR is OK. I recently updated the DNSRecords() method (in #102) to do the pagination itself and return all records, until we've implemented pagination properly.

// OrganizationResponse represents the response from the Organization endpoint.
type OrganizationResponse struct {
// organizationResponse represents the response from the Organization endpoint.
type organizationResponse struct {
Response
Result []Organization `json:"result"`
ResultInfo ResultInfo `json:"result_info"`
Copy link
Contributor

Choose a reason for hiding this comment

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

Go lets you omit the second ResultInfo here but I'm OK either way having it or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed ResultInfo

Signed-off-by: Gábor Lipták <gliptak@gmail.com>
@jamesog jamesog merged commit 9d186a5 into cloudflare:master Jan 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants