Skip to content

Conversation

@muesli
Copy link
Collaborator

@muesli muesli commented Nov 17, 2017

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 muesli requested a review from ChimeraCoder November 17, 2017 16:20
@muesli
Copy link
Collaborator Author

muesli commented Nov 17, 2017

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
Copy link
Owner

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: #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
Copy link
Collaborator Author

muesli commented Feb 11, 2018

Of course, happy to update them!

@muesli
Copy link
Collaborator Author

muesli commented Feb 20, 2018

Pushed. Sorry for the delay!

@muesli
Copy link
Collaborator Author

muesli commented Feb 20, 2018

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
Copy link
Owner

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.

@ChimeraCoder ChimeraCoder merged commit 9c68684 into ChimeraCoder:master Feb 26, 2018
@cention-mujibur-rahman
Copy link
Contributor

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

iwataka added a commit to iwataka/anaconda that referenced this pull request May 5, 2022
* 'master' of https://github.com/ChimeraCoder/anaconda:
  add PostAccountUpdateProfile (ChimeraCoder#259)
  Add RemoveUserFromList / RemoveMultipleUsersFromList (ChimeraCoder#247)
  Update README for streaming API. (ChimeraCoder#241)
  Add Category struct and change GetUsersSuggestions to use it (ChimeraCoder#228)
  Use exclude instead of include directive for Travis (ChimeraCoder#235)
  Implement GetListMembers (ChimeraCoder#240)
  Cleaned up README indentations and captions (ChimeraCoder#237)
  Implement IndicateTyping endpoint (ChimeraCoder#236)
  Enforce a default HTTP client timeout (ChimeraCoder#226)
  Store OAuth credentials per TwitterApi instance (ChimeraCoder#222)
  Disable Go 1.6 tests on OS X in Travis (ChimeraCoder#232)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multiple applications per executable

3 participants