-
Notifications
You must be signed in to change notification settings - Fork 146
Add Lint Warnings to Public def of Lints & emit Warning Logs when using ServiceBuilder #508
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
base: main
Are you sure you want to change the base?
Add Lint Warnings to Public def of Lints & emit Warning Logs when using ServiceBuilder #508
Conversation
…ning to deprecated components & fields Signed-off-by: Jem Davies <[email protected]>
4aed88e
to
ed82191
Compare
} | ||
|
||
// LintLevel describes the severity level of a linting error. | ||
// NOTE: These should be kept in sync with ./internal/docs/field.go:726. |
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.
Maybe make this in in-comment function, since this will be exposed in public
otherwise.
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.
It's the same as https://github.com/warpstreamlabs/bento/pull/508/files#diff-cbeda7f53df6b8a29295c69f3ee48389cfcb2fd5f090f03c36c195285f29c82cR12
Maybe it's not that bad that people will see that - they might check that it's true
lintingDisabled bool | ||
envVarLookupFn func(string) (string, bool) | ||
|
||
lintWarns []Lint |
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.
Does it make sense to store/cache lint warnings? I prefer the approach of linting on build/startup (unsure if this is already happening).
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.
Yeah so it's like this because we lint the configs when we call streambuilder.AddInputYaml
/ .SetYaml
etc.
But at that time we don't necessarily have a logger until we call streambuilder.Build()
where if we haven't set a logger via .SetLogger()
we will get a default, and then we log the cached lintWarns.
Perhaps there is a better way - this is just the first that came to mind...
Signed-off-by: Jem Davies <[email protected]>
No description provided.