Fix invalid JSON struct tag, and other linter issues #319
Conversation
This change is a pretty noisy change in that I went through and cleaned up all of the issues that my linters flagged. I wanted to get this done, so that we can add them as automatic CI checks for future PRs. In the process I did find one critical bug, where we had a malformed JSON struct tag: https://github.com/PagerDuty/go-pagerduty/blob/69ade4b95ef0ff1d582a7d9f226a98f9db886df1/webhook.go#L44 There are two linter issues we cannot fix until v1.5.0, because they are breaking changes. As such, the v1.5.0 development line would be the first place we can enforce those CI checks. Fixes #317
| func (c *Client) TestAbility(ability string) error { | ||
| return c.TestAbilityWithContext(context.Background(), ability) | ||
| } | ||
|
|
||
| // TestAbility Check if your account has the given ability. | ||
| // TestAbilityWithContext checks if your account has the given ability. |
There was a problem hiding this comment.
This was flagged by golint.
| @@ -12,14 +12,13 @@ func TestAbility_ListAbilities(t *testing.T) { | |||
|
|
|||
| mux.HandleFunc("/abilities", func(w http.ResponseWriter, r *http.Request) { | |||
| testMethod(t, r, "GET") | |||
| w.Write([]byte(`{"abilities": ["sso"]}`)) | |||
| _, _ = w.Write([]byte(`{"abilities": ["sso"]}`)) | |||
There was a problem hiding this comment.
This was flagged by errcheck, which finds places where error checking was missed.
| }) | ||
|
|
||
| var client = &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient} | ||
| client := &Client{apiEndpoint: server.URL, authToken: "foo", HTTPClient: defaultHTTPClient} |
There was a problem hiding this comment.
My editor uses gofumpt instead of go fmt for style*, and it's a bit more opinionated. This was changed automatically.
- Technically it's
gofumports, which is meant to replacegoimports.
| @@ -30,14 +30,17 @@ func TestAnalytics_GetAggregatedIncidentData(t *testing.T) { | |||
| } | |||
|
|
|||
| bytesAnalyticsResponse, err := json.Marshal(analyticsResponse) | |||
| testErrCheck(t, "json.Marshal()", "", err) | |||
There was a problem hiding this comment.
The ineffective assignment linter caught that we weren't handling the err value above; this is absolutely a bug.
| @@ -54,6 +54,8 @@ type APIReference struct { | |||
| Type string `json:"type,omitempty"` | |||
| } | |||
|
|
|||
| // APIDetails are the fields required to represent a details non-hydrated | |||
| // object. | |||
There was a problem hiding this comment.
This was caught by golint.
| client := &Client{apiEndpoint: server.URL, | ||
| authToken: "foo", | ||
| HTTPClient: defaultHTTPClient, | ||
| client := &Client{ |
There was a problem hiding this comment.
This change was also gofumports.
|
@stmcallister wanted to ping again to see if you may have a moment to 👀 this week. I apologize for it being such a large PR, thankfully the majority of changes were done by automatic tooling and not by hand. |
This change is a pretty noisy change in that I went through and cleaned up all
of the issues that my linters flagged. I wanted to get this done, so that we can
add them as automatic CI checks for future PRs.
In the process I did find one critical bug, where we had a malformed JSON struct
tag:
go-pagerduty/webhook.go
Line 44 in 69ade4b
There are two linter issues we cannot fix until v1.5.0, because they are
breaking changes. As such, the v1.5.0 development line would be the first place
we can enforce those CI checks.
Fixes #317