wit-ai / wit-go

Go client for wit.ai HTTP API

MIT License

156 stars 30 forks source link

BREAKING: Bump API version to 20200513 #19

Closed nickylogan closed 3 years ago

nickylogan commented 3 years ago

Description

This PR updates the library to support wit.ai's latest API version. See #17.

Pull Requests

Thanks for proposing a pull request.

To help us review the request, please complete the following:

[x] sign contributor license agreement: https://developers.facebook.com/opensource/cla
[x] submit against our master.
[x] describe the change (for example, what happens before the change, and after the change)

facebook-github-bot commented 3 years ago

Hi @nickylogan!

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

plutov commented 3 years ago

@nickylogan thank you for the PR! It looks great, since it's a breaking one there are things we should consider:

Have clear documentation in README on now to use new API version vs old one. Also, do we need to set API version in the client or we'll keep only one? I suggest keeping single API version without letting users change it, since our Go structs support one version only.
Check integration tests
Bump new semver (I'll do this after merge)

nickylogan commented 3 years ago

Also, do we need to set API version in the client or we'll keep only one? I suggest keeping single API version without letting users change it, since our Go structs support one version only.

@plutov The old client has a way to set the version of the API. If that's the case, I shall remove it along with this PR.

plutov commented 3 years ago

Yes, please remove the option to set API version

plutov commented 3 years ago

Looks good, can you also please update the README file on how to init a client?

nickylogan commented 3 years ago

Looks good, can you also please update the README file on how to init a client?

@plutov Will definitely do. But there are still some more changes to be made. I'll update the README after all the code changes.

nickylogan commented 3 years ago

@plutov hmmm. This PR is not ready yet, integration tests didn't work because of docs mismatch with actual API behavior. Are you sure this is intended?

els2520 commented 3 years ago

Description

This PR updates the library to support wit.ai's latest API version. See #17.

Pull Requests

Thanks for proposing a pull request.

To help us review the request, please complete the following:

[x] sign contributor license agreement: https://developers.facebook.com/opensource/cla

[x] submit against our master.

[x] describe the change (for example, what happens before the change, and after the change)