-
Notifications
You must be signed in to change notification settings - Fork 50
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
[SM-1223] restructure Go wrapper for distribution #647
Conversation
New Issues
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #647 +/- ##
=======================================
Coverage 60.53% 60.53%
=======================================
Files 172 172
Lines 10527 10527
=======================================
Hits 6373 6373
Misses 4154 4154 ☔ View full report in Codecov by Sentry. |
@@ -3,7 +3,7 @@ package sdk | |||
import ( | |||
"encoding/json" | |||
|
|||
"github.com/bitwarden/sdk/languages/go/internal/cinterface" | |||
"github.com/bitwarden/sm-sdk-go/cinterface" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The internal package was intentional, to limit the external APIs and reduce the surface for major versions. The cinterface
should not be exposed externally. The reason this doesn't work is because you are trying to import it from the sm-sdk-go repository and it's loading two different packages.
We also don't want to distribute the lib files in a repository unless we force puch every release. Since with the binaries occupying 30mb+ we can expect the repository size to quickly become unreasonable. |
That makes sense, but what's the alternative? Would we require people that want to import our Go wrapper to build the libs from source? |
I'm working on adding Bitwarden secrets manager as a provider to external secret operator for k8s and not being able to use this sdk as a package is a real deal breaker. Do you have any recommendations on how this should be used in a multi architecture environment? And the go example doesn't work, the Command struct is not imported Thank you |
Type of change
Objective
Restructure the Go wrapper so that it works when consumed as a package. The current structure uses "internal" Go modules, which will throw errors when you try to use them:
Code changes
Before you submit