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 Manage Object to Hide Internal Details #107

Closed

Conversation

dvonthenen
Copy link
Contributor

Proposed changes

NOTE: This depends on #86, #87, #88, #103 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/prerecorded-object-definition...dvonthenen:deepgram-go-sdk:dv/manage-object-definition

Addresses #104, #105, #106.

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

  • Fixes it so all REST plumbing is not hidden from the user in the manage functions
  • Implements a example for every single API call within manage
    • examples/balances
    • examples/invitations
    • examples/keys
    • examples/members
    • examples/prerecorded (this was already there)
    • examples/projects
    • examples/scopes
    • examples/streaming (this was already there)
    • examples/usage
  • Implements nesting the HTTP Error code into all errors return from REST calls
  • Fixed Live Client to be similar to Prerecord/Manage when Initializing 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
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/prerecorded-object-definition...dvonthenen:deepgram-go-sdk:dv/manage-object-definition

@SandraRodgers
Copy link
Contributor

I tested the examples and they all work!

SandraRodgers
SandraRodgers previously approved these changes Oct 30, 2023
@dvonthenen dvonthenen changed the base branch from main to go-sdk-v1 October 30, 2023 23:55
@dvonthenen dvonthenen dismissed SandraRodgers’s stale review October 30, 2023 23:55

The base branch was changed.

SandraRodgers
SandraRodgers previously approved these changes Oct 31, 2023
@dvonthenen dvonthenen dismissed SandraRodgers’s stale review October 31, 2023 18:13

The merge-base changed after approval.

@dvonthenen
Copy link
Contributor Author

Rebased and squashed. Ready for review again.

@dvonthenen
Copy link
Contributor Author

going to break this PR into 3-4 smaller PRs. closing.

@dvonthenen dvonthenen closed this Oct 31, 2023
@dvonthenen dvonthenen deleted the dv/manage-object-definition branch November 2, 2023 14:25
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.

2 participants