refactor: Reduce code duplication in analytics.go #397
Conversation
There was a problem hiding this comment.
Thank you for this contribution! I did find two minor things that I requested changes on, but beyond that this should be good to go.
I had originally tagged the issue (#393) to be part of v1.6.0, but since you've opened this PR up so quickly I can make it part of the upcoming v1.5.0 release once the changes are made.
analytics.go
Outdated
| ) | ||
|
|
||
| const baseURL = "/analytics/metrics/incidents" |
There was a problem hiding this comment.
Since constants are local to the package and not just this file, would you be able to make this constant name a little more precise? Maybe analyticsBaseURL?
analytics.go
Outdated
| h := map[string]string{ | ||
| "X-EARLY-ACCESS": "analytics-v2", | ||
| } | ||
|
|
||
| resp, err := c.post(ctx, "/analytics/metrics/incidents/teams", analytics, h) | ||
| URL := fmt.Sprintf("%s/%s", baseURL, endpoint) |
There was a problem hiding this comment.
This is an odd nit, I'll admit, but since capitalized identifiers are meant for things that are exported it feels a little unusual to have this be called URL. Could you rename it to url or even just u?
baseURL -> analyticsBaseURL URL -> u
|
Thank you for reviewing my commits! |
Refer to #393