ChimeraCoder / anaconda

A Go client library for the Twitter 1.1 API
MIT License
1.14k stars 247 forks source link

Decouple TwitterApi from global oauthClient #222

Closed muesli closed 6 years ago

muesli commented 6 years ago

Each TwitterApi now maintains its own oauthClient. The global oauthClient has been replaced by a slimmer global oauthCredentials to maintain backwards-compatibility with the old API.

This fixes #101 and is based on #182.

I'd appreciate a thorough review.

muesli commented 6 years ago

Note: we actually do break the API compatibility for AuthorizationURL and GetCredentials. That being said, I'm not quite sure those methods were ever meant to be exported/public.

None of the public examples / tests rely on it, fwiw.

EDIT: I now understand the need for those methods. We should really document their use-case. This change will break the API for users of this authentication flow.

ChimeraCoder commented 6 years ago

Thanks for waiting on this - since this is the first time we're heard-breaking backwards compatibility in a significant (albeit easily-upgradeable) way, I wanted to make sure we were doing everything properly with respect to vendoring and versioning (see also: https://github.com/ChimeraCoder/anaconda/issues/230).

Don't worry about the build failure - that's because the new OS X image for Go 1.6 on Travis is buggy (I've encountered this with my other projects too). I'll need to disable Travis builds for Go 1.6 on OS X - I'll do that on a separate PR.

All this looks good to me. Would you mind updating the first example in example_test.go and the first sample in the README to go along with it?

muesli commented 6 years ago

Of course, happy to update them!

muesli commented 6 years ago

Pushed. Sorry for the delay!

muesli commented 6 years ago

If we're breaking backwards compatibility, we could have a discussion if there are other breaking changes that might also be worth being included in the next release. A couple linter warnings come to mind.

Otoh, this change alone probably doesn't affect too many of anaconda's current users (I'm just guessing, really, I have no figures that would back me up, obviously).

ChimeraCoder commented 6 years ago

Whoops - github didn't give me a notification for your first comment and commit, just the second one. Thanks for pushing it!

Now that we have a vendoring and tagging strategy that's flexible enough, I'm comfortable making breaking changes more regularly (and tagging them appropriately), so we can address those separately.

cention-mujibur-rahman commented 6 years ago

Hello @muesli, Sorry I'm very late to this discussion. It looked like the breaking change reasonable. But how can we handle the situation while doing the authentication? During authentication time, say still I didn't get the access_token, access_token_secret yet and there its needed the AuthorizationURL to be called to get the temporary credentials.

Thanks