-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
add lint test:-confusing-naming #5507
Conversation
Signed-off-by: Your Name <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5507 +/- ##
==========================================
+ Coverage 96.20% 96.21% +0.01%
==========================================
Files 327 327
Lines 16011 16011
==========================================
+ Hits 15403 15405 +2
+ Misses 434 433 -1
+ Partials 174 173 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
there are no lint rules that are currently failing on the main branch. Where do these come from? |
when I run
|
Signed-off-by: Your Name <[email protected]>
Signed-off-by: Your Name <[email protected]>
you may have different version of the tools, try |
Signed-off-by: Your Name <[email protected]>
@@ -44,7 +44,7 @@ var _ component.Host = (*otelHost)(nil) // API check | |||
// StartOTLPReceiver starts OpenTelemetry OTLP receiver listening on gRPC and HTTP ports. | |||
func StartOTLPReceiver(options *flags.CollectorOptions, logger *zap.Logger, spanProcessor processor.SpanProcessor, tm *tenancy.Manager) (receiver.Traces, error) { | |||
otlpFactory := otlpreceiver.NewFactory() | |||
return startOTLPReceiver( | |||
return start_OTLP_Receiver( |
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.
This is not what the linter was complaining about. Your names violate Go coding practices.
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.
so how should i fix this naming problem (Warns on methods with names that differ only by capitalization) ?
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.
will it be okay if i name it like startOTLPReceiverInternal
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.
I think it is fine to use startOTLPReceiver as of camelCase which is generally used in Go.
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.
startOTLPReceiverInternal
seems ok, but this needs to be decided on a case by case basis, there may be better options in other situations.
Most of the changes proposed are much worse than what we have. |
Which problem is this PR solving?
Description of the changes
make lint
check that was failingHow was this change tested?
Checklist
jaeger
:make lint test
jaeger-ui
:yarn lint
andyarn test