-
Notifications
You must be signed in to change notification settings - Fork 26
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
API for the Aggregator Endpoint #24
base: master
Are you sure you want to change the base?
Conversation
By default we only receive the review_type_ids. This way we can get information about the reviews, such as their name. There does not appear to be a separate endpoint to get this information. This works by using the "response customization" feature of the API.
Fill in review_types when listing reviews
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had a few small comments.
v5/pivotal/aggregator.go
Outdated
return &AggregatorService{client} | ||
} | ||
|
||
// Adds the url for the story to the aggregation data. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is idiomatic to start the documentation with the name of the thing you are documenting , e.g., this might be "Story adds the url for the story to the aggregation data."
Would you be willing to update this for all the public member in this PR?
v5/pivotal/aggregator.go
Outdated
return a | ||
} | ||
|
||
func BuildStoryURLOnlyUsingStoryID(storyID int) string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do these URL helpers need to be public? I think they would only be used internally.
v5/pivotal/aggregator.go
Outdated
} | ||
|
||
// Sends the request for the aggregation. | ||
func (a *Aggregation) Send() (*Aggregation, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like all of the other equivalent methods in the module return the *http.Response
as well, e.g., List()
. It'd probably make sense to do that for consistency. I am not sure the exact use case.
Adding Accounts Service
Add requested_by_id field
Hi,
This package currently doesn't support the aggregator endpoint, which is super useful at decreasing the amount of requests that could be made to PivotalTracker.
https://www.pivotaltracker.com/help/api/#Using_the_GET_Request_Aggregator
This PR adds a new service to the package for using the aggregator. It currently only supports getting the stories, comments of stories and reviews of stories. However, if the pattern is approved, I believe the API could be extended to support other parts of PivotalTracker.
Note: It uses some parts from the fork https://github.com/oschwald/go-pivotaltracker.
A sample use would be similar to below,