Closed OoLunar closed 2 weeks ago
[!CAUTION]
Review failed
The pull request is closed.
This pull request encompasses a comprehensive update to the project's structure, including the deletion of several Visual Studio solution and project files, specifically Deepgram.Dev.sln
, Deepgram.DevBuild.sln
, and various .csproj
files. Additionally, it introduces a new .gitignore
file with an expanded list of ignored files and directories relevant to various development tools and environments. The project files that remain have been updated to target .NET 9.0 and include new dependencies, reflecting a shift towards more modern configurations.
File/Directory Path | Change Summary |
---|---|
.gitignore |
Expanded list of ignored files for development environments, including Visual Studio, .NET, etc. |
Deepgram.Dev.sln |
Deleted; was a Visual Studio solution file for the Deepgram project. |
Deepgram.DevBuild.sln |
Deleted; was a Visual Studio solution file for the Deepgram build. |
Deepgram.Microphone/Deepgram.Microphone.csproj |
Updated package references to fixed versions for .NET frameworks. |
Deepgram.Tests/Deepgram.Tests.csproj |
Updated package references with newer versions. |
Deepgram.sln |
Significant restructuring with updated project identifiers and new example projects added. |
Deepgram/Deepgram.csproj |
Updated package references for various dependencies. |
Directory.Build.props |
New file created to define project root property. |
clean-up.sh |
Deleted; was a shell script for cleaning project directories. |
Various examples/.../*.csproj |
New project files created for various examples, targeting .NET 9.0 with updated dependencies. |
Various examples/.../Program.cs |
New C# programs introduced with updated namespaces and accessibility for main methods. |
examples/analyze/.../Analyze.csproj |
Deleted; project files related to analyze functionality removed. |
examples/manage/.../Manage.csproj |
Deleted; project files related to manage functionality removed. |
examples/speech-to-text/.../PreRecorded.csproj |
Deleted; project files related to speech-to-text functionality removed. |
examples/text-to-speech/.../Speak.csproj |
Deleted; project files related to text-to-speech functionality removed. |
.gitignore
file to include various development tools and environments, including those related to TTS..gitignore
updates.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?
Some comments:
@OoLunar
the PR is very large to review for the team and this approach to refactor the code base isn't what we are looking for at this time.
the unit tests are broken: https://github.com/deepgram/deepgram-dotnet-sdk/actions/runs/11692426812/job/32562168995?pr=352
the build is broken: https://github.com/deepgram/deepgram-dotnet-sdk/actions/runs/11692426812/job/32562168134?pr=352
I am guessing the upload to nuget is also broken.
The docs would need to be fixed in multiple places, which isn't something we want to do right now.
I think it be best to close this out as our main goal should be be to maintain and support the existing SDK, not focus on a large refactor at this time.
the PR is very large to review for the team and this approach to refactor the code base isn't what we are looking for at this time. [...] I think it be best to close this out as our main goal should be be to maintain and support the existing SDK, not focus on a large refactor at this time.
That's entirely okay. I still think we should update the dependencies at minimum to address the STJ vulnerability, however the rest of this work can be done again on a different day. I'll save this branch for later, nothing a bit of rebase magic can't help with.
@OoLunar if the dependency change won't introduce anything breaking to our users we can consider it.
Proposed changes
This PR does the following:
examples
andtests
directories per .NET convention.gitignore
fileTypes of changes
What types of changes does your code introduce to the community .NET 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
This PR is the first of many for updating this repository to meet the standard .NET project layout and practices. The next PR will be cleaning up the example and test code, then using the conventional logging from
Microsoft.Extensions.Logging.Abstractions
.https://wakatime.com/@a8e4b5db-b8cc-469a-acce-099a0a2425be/projects/dammoujrda?branches=oolunar%2Fupdate-infra&start=2024-10-31
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Documentation