Closed dvonthenen closed 2 months ago
[!IMPORTANT]
Review skipped
More than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review.
60 files out of 129 files are above the max files limit of 50. Please upgrade to Pro plan to get higher limits.
You can disable this status message by setting the
reviews.review_status
tofalse
in the CodeRabbit configuration file.
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
Proposed changes
Addresses: https://github.com/deepgram/deepgram-go-sdk/issues/242
Ahead of the TTS WS release, we temporarily placed the TTS WS implementation at
pkg/api/speak-stream
andpkg/client/speak-stream
in this PR: https://github.com/deepgram/deepgram-go-sdk/pull/239 to avoid a naming collision between the REST and WebSocket implementation. This PR puts the package in a more thought-out and permanent location before we release the feature.This looks like a lot, but I tried to commit
git mv
so that it can keep track of these changes easier. In most cases, it seems to have done a decent job. In other cases, it seems to have not done so well. The overwhelming majority of the other changes include fixing things with static checks, moving things, and so on.pkg/client
ChangesBecause the clients were NOT versioned, this enabled the process to do that. To demonstrate the approach for
clients
on this PR, the following changes were made to the clients.The way it was before:
The files were moved into a
pkg/client/<type>/v1
folder and thepkg/client/<type>
created backward-compatible links to artifacts in these newv1
folders.pkg/api
ChangesThe
pkg/api
were versioned, but we need to change theapi
s so thatspeak
makes a lot more sense. We don't want aspeak
andspeak-stream
API package. The implementation of TTS WS was done purposely so that this change could be made easily. These changes then follow the client in a similar fashion.The way it was before:
The files were moved into a
pkg/api/<type>v1
folder and thepkg/client/<type>
created backward-compatible links to artifacts in these newv1
folders.The README was updated to keep the idea of Live/Stream and PreRecorded, but when it goes out to code, it maps to
listen/rest
andlisten/websocket
. The same is true forspeak
.The users' code should remain unchanged, but now hint that there are deprecated packages in use to be removed at the next major release. This is what it will look like in the IDE/editor by way of a strikethrough (picture below) and this warning in the deprecated packages:
Any examples with the name
new-RENAME
(for example:examples/text-to-speech/rest/file-new-RENAME/hello-world/main.go
) is using the new package names as to demonstrate the deprecated and new ways can both coexist.Other notable changes:
PackageVersion
was placed on all API and Client packageslive
->listenv1ws
.TODOs:
examples
other than renaming the folders to demonstrate the backward compatibility of these changes. In a follow-up PR, I will change all the examples to be implement without using these deprecated package references so people are set on the right path on how to use the packages in this repo.Types of changes
What types of changes does your code introduce to the community Go SDK? Put an
x
in the boxes that applyChecklist
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.Further comments
NA