Skip to content

Conversation

@mfb2
Copy link
Contributor

@mfb2 mfb2 commented Aug 5, 2019

This commit adds the ability to fetch a list of branches from Bitbucket
via pagination.

This commit adds the ability to fetch a list of branches from Bitbucket
via pagination.
}

urlStr := r.c.requestUrl("/repositories/%s/%s/refs/branches?%s", rbo.Owner, rbo.RepoSlug, params.Encode())
response, err := r.c.executeRaw("GET", urlStr, "")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The existing call to execute presumed pagination, but for some reason came back malformed. I opted to manually map the data on the response instead.

page, ok := branchResponseMap["page"].(float64)
if !ok {
page = 0
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm curious as to your thoughts on this section. This set of conditionals that evaluate the metadata (page, pagelen, size) in the response from Bitbucket should always be valued; however, if something is missing, the current behavior will default it to a value of 0. I'm not certain how consumers would treat or view this response; any insights on a possibly better idea?

Copy link
Owner

Choose a reason for hiding this comment

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

It is no problem, I think. At the struct definition, 0 is correct If you use int types as Page int.
(https://github.com/ktrysmt/go-bitbucket/pull/66/files#diff-ab2c7209d94fb144c8d55246ad821816R47)

So gopher would be care to use to struct and zero types. @mfb2

@mfb2
Copy link
Contributor Author

mfb2 commented Aug 5, 2019

Hello! I tested this and it works well for me, though I'd sleep much better if someone else could validate/confirm. Please let me know if you have any feedback or questions, I'm happy to help out!

Manny Batule added 2 commits August 6, 2019 10:22
This commit converts a pointer to branches to an actual object reference
in order to keep the response immutable.
Adds ability to specify number of records to be returned on each page.
Copy link
Owner

@ktrysmt ktrysmt left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!

page, ok := branchResponseMap["page"].(float64)
if !ok {
page = 0
}
Copy link
Owner

Choose a reason for hiding this comment

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

It is no problem, I think. At the struct definition, 0 is correct If you use int types as Page int.
(https://github.com/ktrysmt/go-bitbucket/pull/66/files#diff-ab2c7209d94fb144c8d55246ad821816R47)

So gopher would be care to use to struct and zero types. @mfb2

@mfb2
Copy link
Contributor Author

mfb2 commented Aug 8, 2019

Thank you for approving! I am happy to contribute, and there's a chance I may submit a few more improvements in the near future. Please let me know if I can provide any other assistance – I look forward to the merge! 😄

@ktrysmt ktrysmt merged commit df6d5a4 into ktrysmt:master Aug 9, 2019
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.

2 participants