-
Notifications
You must be signed in to change notification settings - Fork 842
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
refactor(instrumentation-grpc): fix eslint warnings #5408
refactor(instrumentation-grpc): fix eslint warnings #5408
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5408 +/- ##
=======================================
Coverage 94.64% 94.64%
=======================================
Files 318 318
Lines 8033 8033
Branches 1688 1688
=======================================
Hits 7603 7603
Misses 430 430
|
``` /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-grpc/src/clientUtils.ts 105:42 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-grpc/src/instrumentation.ts 129:69 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any 134:69 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any 139:68 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any 144:68 warning Unexpected any. Specify a different type @typescript-eslint/no-explicit-any /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-grpc/src/serverUtils.ts 112:23 warning Don't use `Function` as a type @typescript-eslint/ban-types 152:23 warning Don't use `Function` as a type @typescript-eslint/ban-types 207:31 warning Don't use `Function` as a type @typescript-eslint/ban-types 211:31 warning Don't use `Function` as a type @typescript-eslint/ban-types /home/runner/work/opentelemetry-js/opentelemetry-js/experimental/packages/opentelemetry-instrumentation-grpc/test/helper.ts 83:11 warning Don't use `Function` as a type @typescript-eslint/ban-types ``` This commit does not involve any changes to runtime code and can be safely merged without any concerns about changing behavior. The first can be replaced with the more precise type that we are expecting. The second just wansn't needed anymore. The third is highly unusual. After spending significant amount of time reading and stepping through the code – I am quite confident that the `.call({}, ...)` (and `.apply({}, ...)` elsewhere in the tests) are a mistake that we inherited from very old code. However I don't feel confident enough that I can *explain* what is going on here to make a change to the runtime code, so in the meantime, I think the best course of action is to leave it as-is, document the issue and have someone with more expertise untangle this down the road. The last one was a change in test code to use a more precise call signature. Ref open-telemetry#5365
860134a
to
ffae1a2
Compare
@chancancode it's failing because it requires a changelog entry. Either you:
my preference is the 1st option. Same applies to the other eslint PRs you created |
So far we have skipped chanelogs for all of the refactors related to #5365 🙂 the changes are probably too inconsequential for the end users of the packages |
b5885e9
Which problem is this PR solving?
Fixes the following eslint warnings in the instrumentation-grpc package:
Ref #5365
Short description of the changes
This commit does not involve any changes to runtime code and can be safely merged without any concerns about changing behavior.
The first can be replaced with the more precise type that we are expecting.
The second just wansn't needed anymore.
The third is highly unusual. After spending significant amount of time reading and stepping through the code – I am quite confident that the
.call({}, ...)
(and.apply({}, ...)
elsewhere in the tests) are a mistake that we inherited from very old code. However I don't feel confident enough that I can explain what is going on here to make a change to the runtime code, so in the meantime, I think the best course of action is to leave it as-is, document the issue and have someone with more expertise untangle this down the road.The last one was a change in test code to use a more precise call signature.
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Checklist: