Fix regression caused by resolveOptions

[chelnak]

If opts was nil, resolveOptions would only update the local copy of opts. The pointer was never being updated and was therefore nil when passed to the New*Client methods.

Closes #17

[chelnak]

Hey, that sounds good. I'll push some changes + new tests this afternoon.

Cheers, Craig

[chelnak]

@samcoe I was wondering if it was still desirable to allow the consumer to pass nil for ClientOptions.

e.g: gh.HTTPClient(nil)

Then it would be up to the client methods to pass a valid api. ClientOptions.

func HTTPClient(opts *api.ClientOptions) (*http.Client, error) {
    if opts == nil {
        opts = &api.ClientOptions{}
    }
.....

The other option would be to ask he consumer to provide an empty api.ClientOptions.

[samcoe]

My thought would be that it should be up to the client methods to pass a valid api.ClientOptions to resolveOptions even if they received nil, so exactly as you proposed 👍

[chelnak]

I've pushed an implementation. I couldn't decide whether to include a nil check in resolveOptions so I left it out. I don't think it will ever be an issue to the consumer given that the client methods now handle the opts. Saying that though.. I'm more than happy to add something in if it's preferable!

Currently pondering on tests... it feels tricky because of config.Load() and mocking it would mean exporting some more things from the config package..

or we could pass in config to resolveOptions e.g

func resolveOptions(clientOptions **api.ClientOptions, cfg config.Config) error {
...

however I'm fairly fresh with Go so I might be overlooking something obvious.

[chelnak]

Added some tests and rebased 👍

[chelnak]

@samcoe appreciated 😄