Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Encapsulate the Prerecorded Object to Hide Internal Details #103

Merged

Conversation

dvonthenen
Copy link
Contributor

@dvonthenen dvonthenen commented Oct 24, 2023

Proposed changes

NOTE: This depends on #86, #87, #88 and is targeted to go-sdk-v1 release. Will rebase when dependent PRs are merged. The diff in this PR will look pretty nasty until subsequent depend PRs are merged first. Until these get merged, here is a better looking diff to track the changes between branches: dvonthenen/deepgram-go-sdk@dv/live-streaming-object-definition...dvonthenen:deepgram-go-sdk:dv/prerecorded-object-definition

Addresses #92, #102, and #105.

Implements more of an object-oriented approach to handling the prerecorded struct. This change implements the following:

  • Separates the REST Client and Prerecorded Client in separate packages
  • Separates Prerecorded Client and API Bindings into separate packages
  • Requires a Go context to be passed into the Live Client creation. This is to facilitate:
    • Async Cancel for any REST call
  • The examples/prerecorded has been updated to show how to use the client.

Types of changes

What types of changes does your code introduce to the community Go SDK?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update or tests (if none of the other choices apply)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I have read the CONTRIBUTING doc
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in downstream modules

Further comments

PR #86 handled the restructure of the project. This is the first PR to start refactoring and improving individual components in this new restructure. This PR handles just the Live Client and Live Client only. I am trying to reduce the amount of code change in this PR and each component will be handled one at a time.

Will squash the commits after the review is completed.

@dvonthenen dvonthenen force-pushed the dv/prerecorded-object-definition branch 3 times, most recently from 990bc96 to 4d28e39 Compare October 25, 2023 00:11
@dvonthenen
Copy link
Contributor Author

EDIT: updated description to include Issue #105 being addressed by this PR

@dvonthenen
Copy link
Contributor Author

Until these get merged, here is a better looking diff to track the changes between branches: dvonthenen/deepgram-go-sdk@dv/live-streaming-object-definition...dvonthenen:deepgram-go-sdk:dv/prerecorded-object-definition

lukeocodes
lukeocodes previously approved these changes Oct 27, 2023
@SandraRodgers
Copy link
Contributor

I get an error when I run the project main.go file on this branch.

Copy link
Contributor

@SandraRodgers SandraRodgers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you double check the examples/projects/main.go? I'm getting an error when I run that file on this branch

@SandraRodgers SandraRodgers self-requested a review October 30, 2023 15:27
SandraRodgers
SandraRodgers previously approved these changes Oct 30, 2023
@dvonthenen
Copy link
Contributor Author

Can you double check the examples/projects/main.go? I'm getting an error when I run that file on this branch

discussed in slack... I didn't really treat the projects example as passable until the manage API was refactored here:
#107

at that point, I was just using that example as testing grounds for various APIs.

@dvonthenen dvonthenen changed the base branch from main to go-sdk-v1 October 30, 2023 23:55
@dvonthenen dvonthenen dismissed stale reviews from SandraRodgers and lukeocodes October 30, 2023 23:55

The base branch was changed.

@dvonthenen dvonthenen force-pushed the dv/prerecorded-object-definition branch from 4d28e39 to 94e383c Compare October 30, 2023 23:58
@dvonthenen dvonthenen force-pushed the dv/prerecorded-object-definition branch from 94e383c to 9e15ccf Compare October 31, 2023 14:14
@dvonthenen
Copy link
Contributor Author

Can you double check the examples/projects/main.go? I'm getting an error when I run that file on this branch

I deleted the examples/projects until it is fixed in the manage refactor. examples/prerecorded and examples/streaming should be working though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants