babbel / rollbar-go Goto Github PK

Should we maybe add a test which tries to authorize with the api and fail if it's not able to do so?

Currently, the API resources correspond to one file each which is natural for an SDK. However, the files define methods on the Client. Instead they should define methods on the respective struct.

Example: users.go defines InviteUser() on Client which is in client.go.

It would seem more natural to have a struct for each API resource. In the example, a User struct would contain the client and have a method User.Invite(). All methods for users would be encapsulated with a User struct. The resource specific struct defines things such as its API endpoint.

For convenience, the client can delegate the methods back or an instance of each struct.

Example SDK

• https://github.com/google/go-github/blob/master/github
• https://github.com/Ableton/go-travis

Convenience function that gets the fixture and sets the handler so that we don't have to repeat mux.HandleFunc

When making a request we currently build a string by hand, then create a http.request, then hand it to the Client's makeRequestmethod.

Simplified example:

url := fmt.Sprintf("%steam/%d/invites?access_token=%s&page=%d", c.ApiBaseUrl, teamID, c.ApiKey, pageNum)
req, err := http.NewRequest("GET", url, nil)
bytes, err := c.makeRequest(req)

The Client should abstract away from low level http objects and handle the composition of the request. Only high-level things should still be handled by the user of the Client. So the above example should look more like this:

params = map[string]string{"teamID":teamID, "page":pageNum}
result, err := Client.Get("invites", params)

This also addresses the issue of makeRequest returning bodies and handling response codes for all http methods, which is wrong and only works for the current small set of methods. Instead, it would be nicer to have one client method for each http method: Get for GET, Delete for DELETE etc.