-
Notifications
You must be signed in to change notification settings - Fork 3
[PM-18071] Server sdk prep for inclusion #88
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #88 +/- ##
==========================================
- Coverage 37.06% 36.75% -0.31%
==========================================
Files 37 36 -1
Lines 1071 1012 -59
Branches 91 86 -5
==========================================
- Hits 397 372 -25
+ Misses 642 609 -33
+ Partials 32 31 -1 ☔ View full report in Codecov by Sentry. |
|
Great job, no security vulnerabilities found in this Pull Request |
Co-authored-by: Daniel García <[email protected]>
|




🎟️ Tracking
📔 Objective
When attempting to integrate the SDK into the
CoreandApiprojects inbitwarden/serverI ran into a couple problems with how the SDK was constructed. Before, we were including theIFeatureServiceinto the compilation of each project but that has the problem where something like whereApireferencesCorewill get two copies ofIFeatureServicethat will break dependency injection so we need there to be a single reference. To do that we need it to be pre-built into an assembly, so this puts all the features code into a new nuget packageBitwarden.Server.Sdk.Features, it's not expected that anyone would use it directly but there is nothing keeping that from working. Now only a single reference of this service will exist and DI will work fine.I also opted for not including the
Bitwarden.Server.Sdkentrypoint i.eUseBitwardenSdkin non web sdk projects. Various parts in that file rely on packages that are implicitly included in the web sdk. Instead of just forcing those packages on consumers directly it makes more since to just leave out the entrypoint and instead allow them to reference things likeIFeatureServiceonly.The main brunt of changes is to allow the tests to reference a fake nuget feed including the
Bitwarden.Server.Sdk.Featuresnuget package.⏰ Reminders before review
🦮 Reviewer guidelines
:+1:) or similar for great changes:memo:) or ℹ️ (:information_source:) for notes or general info:question:) for questions:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion:art:) for suggestions / improvements:x:) or:warning:) for more significant problems or concerns needing attention:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt:pick:) for minor or nitpick changes