feat:Split the loggerfx into a separate internal package before split… #4
Conversation
…ting it to an external library.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4 +/- ##
==========================================
+ Coverage 24.39% 29.48% +5.09%
==========================================
Files 16 16
Lines 574 675 +101
==========================================
+ Hits 140 199 +59
- Misses 416 458 +42
Partials 18 18
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
|
||
| // Module is function that builds the loggerfx module based on the inputs. If | ||
| // the configPath is not provided then the default path is used. | ||
| func Module(configPath ...string) fx.Option { |
There was a problem hiding this comment.
This is a good pattern, but not something we want to import into every application. This is the kind of code that belongs in each application that uses these libraries.
The main reason for this is you cannot know a priori how an application is going to organize its modules. An application might put each http.Server in its own module, for example.
|
|
||
| // DefaultConfig is a helper function that creates a default sallust configuration | ||
| // for the logger based on the application name. | ||
| func DefaultConfig(appName string) sallust.Config { |
There was a problem hiding this comment.
This is a good convention, but it should stay a convention. I'm ok with this being in skeleton, but we should express a dependency on this library via go.mod
| // | ||
| // Make sure to include this option last in the fx.Options list, so that the | ||
| // logger is the last component to be shutdown. | ||
| func SyncOnShutdown() fx.Option { |
There was a problem hiding this comment.
I'm not sure what value this provides, since sallust provides the option.
…ting it to an external library.