Closed joonas-fi closed 4 years ago
p.s. not related to this issue, but I noticed that in api.go
there are several <make http request> + <check response code> + <decode JSON>
blocks like this:
resp, err := c.HTTPClient.Get(u)
if err != nil {
return nil, fmt.Errorf("performing API request: %v", err)
}
defer resp.Body.Close()
// TODO: handle HTTP errors, esp. rate limiting, a lot better
if resp.StatusCode != http.StatusOK {
return nil, fmt.Errorf("HTTP error: %s: %s", u, resp.Status)
}
var tweets []tweet
err = json.NewDecoder(resp.Body).Decode(&tweets)
if err != nil {
return nil, fmt.Errorf("reading response body: %v", err)
}
Just a suggestion (mull it over - no need to decide anything now), I wrote a small helper github.com/function61/gokit/ezhttp
that shortens the above code to:
var tweets []tweet
// ezhttp internally checks for 2xx response code, unless given ezhttp.TolerateNon2xxResponse
// ezhttp returns HTTP response, but usually you don't need it if you let ezhttp check response code + decode JSON
if _, err := ezhttp.Get(context.TODO(), u, ezhttp.RespondsJson(&tweets, true), ezhttp.Client(c.HTTPClient)); err != nil {
return nil, err
}
return tweets, nil
Yes, of course I'm good with merge :)
Excellent, thank you again!! This is awesome. I love that it didn't introduce any new dependencies.
No problem, glad I could be of help. :)
p.s. not meaning to badger or push you, but you mentioned before that you didn't see some GitHub email notifications, so I'll just quickly mention a few issues I created in case you missed the notifications: https://github.com/mholt/timeliner/issues/created_by/joonas-fi (please don't feel obligated to investigate them if you got the notifications but instead are short on time)
https://github.com/mholt/timeliner/issues/25
I ran
$ go fmt ./...
and$ go test ./...