Move Features to SDK #70
Conversation
| @@ -0,0 +1,27 @@ | |||
| <Project Sdk="Microsoft.NET.Sdk.Web"> | |||
|
|
|||
| <Import Project="../../src/Sdk/Sdk.props" /> | |||
There was a problem hiding this comment.
<Import props />
...
<Import targets />Is what a <Sdk /> reference does for you under the hood so this is a good test of the SDK.
|
No New Or Fixed Issues Found |
| @@ -198,6 +146,7 @@ private static void AddSelfHostedConfig(IConfigurationBuilder configurationBuild | |||
|
|
|||
| private static void AddLogging(IServiceCollection services, IConfiguration configuration, IHostEnvironment environment) | |||
There was a problem hiding this comment.
💭 With the logic shift into these methods and since logging, metrics, etc. might not actually get added, should the methods be named differently? Not sure what the norm is here, but you're not authoritatively "adding" one of these with the method call now.
There was a problem hiding this comment.
They are private methods only used within the class so I chose the "easy" path. I either need to add a compile time check around the entire method and in the two places it's called or just do this and leave a no-op. So I chose this only because 1 < 3.
|
Just looked at the CI -- probably want to drop in a preliminary PR with a |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #70 +/- ##
==========================================
- Coverage 37.88% 37.06% -0.83%
==========================================
Files 38 37 -1
Lines 1127 1071 -56
Branches 99 91 -8
==========================================
- Hits 427 397 -30
+ Misses 666 642 -24
+ Partials 34 32 -2 ☔ View full report in Codecov by Sentry. |
🎟️ Tracking
📔 Objective
Move the
Bitwarden.Extensions.Hosting.FeaturesintoBitwarden.Server.Sdk.Features. I slightly changed the layout of the SDK so that it can also get referenced locally. Also since the files are actually a part of the complilation of the consuming project usinginternaldoesn't hide things from them. Instead I do my best to hide it and discourage its use with an[Obsolete]attribute. Long term, stabilized features should probably be moved into their own smaller nuget package. That would allow them to keepinternalin their own assembly and define their own package references. Then theBitwarden.Server.Sdkis just an orchestrator of features to packages.⏰ 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