Remove icon service dependency on core #77
Conversation
…ouple-icon-service # Conflicts: # bitwarden-server.sln # src/EventsProcessor/Startup.cs # src/Icons/Icons.csproj # src/Icons/Startup.cs # src/Notifications/Notifications.csproj # src/SharedWeb/Utilities/ServiceCollectionExtensions.cs
…s never logged currently
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
30 file(s) reviewed, 7 comment(s)
Edit PR Review Bot Settings | Greptile
| @@ -16,7 +16,7 @@ public DateTime GetAlive() | |||
| [HttpGet("~/version")] | |||
| public JsonResult GetVersion() | |||
| { | |||
| return Json(CoreHelpers.GetVersion()); | |||
| return Json(VersionHelper.GetVersion()); | |||
There was a problem hiding this comment.
logic: VersionHelper.GetVersion() is now used instead of CoreHelpers.GetVersion(). Verify that VersionHelper provides the same functionality and returns the version in the expected format.
| @@ -1,9 +1,7 @@ | |||
| using System; | |||
There was a problem hiding this comment.
style: Consider adding XML comments for public methods and classes to maintain documentation standards.
| { | ||
| "Logging": { | ||
| "IncludeScopes": false, | ||
| "LogLevel": { | ||
| "Default": "Debug", | ||
| "System": "Information", | ||
| "Microsoft": "Information" | ||
| }, | ||
| "Console": { | ||
| "IncludeScopes": true, | ||
| "LogLevel": { | ||
| "Default": "Warning", | ||
| "System": "Warning", | ||
| "Microsoft": "Warning", | ||
| "Microsoft.Hosting.Lifetime": "Information" | ||
| } | ||
| "Serilog": { | ||
| "MinimumLevel": { | ||
| "Default": "Error" | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
style: Ensure that this Serilog configuration is consistent with other services and provides sufficient logging information for production troubleshooting.
| } | ||
| "Serilog": { | ||
| "MinimumLevel": { | ||
| "Default": "Error", |
There was a problem hiding this comment.
style: Default log level set to 'Error'. Verify this won't suppress important logs that were previously captured at 'Warning' level.
| @@ -90,5 +90,11 @@ | |||
| }, | |||
| "IpRateLimitPolicies": { | |||
| "IpRules": [] | |||
| }, | |||
| "Serilog": { | |||
| "Enrich": [ "FromLogContext" ], | |||
There was a problem hiding this comment.
style: Consider adding more enrichers like 'WithMachineName' and 'WithThreadId' for more detailed logs
Type of change
Objective
Initial effort to decouple icon service from core, and instead use a
SharedKernelwhich should be versioned as a Nugget package.Remaining work: Remove
LoggerFactoryExtensionfrom remaining projects, and document how to configure the different logger sinks.Code changes
Testing requirements
Before you submit
dotnet tool run dotnet-format --check) (required)Greptile Summary
This pull request introduces changes to decouple the icon service from the core application by implementing a SharedKernel.