Skip to content

fix unused linter errors#8851

Merged
knative-prow[bot] merged 1 commit intoknative:mainfrom
simkam:linter-fix
Jan 26, 2026
Merged

fix unused linter errors#8851
knative-prow[bot] merged 1 commit intoknative:mainfrom
simkam:linter-fix

Conversation

@simkam
Copy link
Contributor

@simkam simkam commented Jan 20, 2026

Can be seen for example in: https://github.com/knative/eventing/actions/runs/21171606060/job/60889521311

  Error: pkg/requestreply/ingress_handler.go:352:6: func isResponseEvent is unused (unused)
  func isResponseEvent(event *cloudevents.Event, rr *v1alpha1.RequestReply) bool {
       ^
  Error: test/conformance/helpers/tracing_test_helper.go:53:3: const recordEventsPodName is unused (unused)
  		recordEventsPodName = "recordevents"
  		^
  Error: test/conformance/helpers/tracing_test_helper.go:98:6: func getTraceIDHeader is unused (unused)
  func getTraceIDHeader(t *testing.T, evInfos []recordevents.EventInfo) string {
       ^
  3 issues:
  * unused: 3

/kind cleanup

@knative-prow knative-prow bot added area/test-and-release Test infrastructure, tests or release size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 20, 2026
@codecov
Copy link

codecov bot commented Jan 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 51.15%. Comparing base (4303f77) to head (aeadfd1).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8851      +/-   ##
==========================================
+ Coverage   51.12%   51.15%   +0.03%     
==========================================
  Files         409      409              
  Lines       21369    21366       -3     
==========================================
+ Hits        10925    10930       +5     
+ Misses       9592     9584       -8     
  Partials      852      852              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@simkam
Copy link
Contributor Author

simkam commented Jan 21, 2026

/retest-required

// }
// t.Fatalf("FAIL: No traceid in %d messages: (%v)", len(evInfos), evInfos)
// return ""
// }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to not commit commented code with a TODO because it won't be picked up. It's better to remove this entire section and replace it with a comment with a reference to an upstream help-wanted ticket which describes why this was commented and what to do about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing is that the code with TODO is already commented (https://github.com/knative/eventing/blob/main/test/conformance/helpers/tracing_test_helper.go#L59) but there are one const definition and one function definition that are used by only that commented code and due to that the unused linting error is triggered. I only commented pieces that are used by that already commented code.

I would rather not remove code that is supposed to be used somewhere. I don't haven't any context why that piece of code was commented.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thing is that the code with TODO is already commented...

Yep, I am aware of that. My intention was to motivate a small clean up of what was there and needs a cleanup because you touch the code in that area anyway. And the reason for the proposed cleanup is what you said a few sentences later =>

I would rather not remove code that is supposed to be used somewhere. I don't haven't any context why that piece of code was commented.

You describe the problem right here perfectly 😆 . That is the reason to not comment and commit code, because nobody will remember why this was done and will be afraid to remove it. For that reason a ticket with a reference should be the way to go. If you don't want to remove it, it's ok but maybe create a ticket and reference it there and the ticket should be "investigate if the commented code is needed or can safely be removed" so it has the chance of being picked up.

Copy link
Contributor Author

@simkam simkam Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, I agree. I logged #8853 and removed the commented code altogether.

Can be seen for example in: https://github.com/knative/eventing/actions/runs/21171606060/job/60889521311

```
  Error: pkg/requestreply/ingress_handler.go:352:6: func isResponseEvent is unused (unused)
  func isResponseEvent(event *cloudevents.Event, rr *v1alpha1.RequestReply) bool {
       ^
  Error: test/conformance/helpers/tracing_test_helper.go:53:3: const recordEventsPodName is unused (unused)
  		recordEventsPodName = "recordevents"
  		^
  Error: test/conformance/helpers/tracing_test_helper.go:98:6: func getTraceIDHeader is unused (unused)
  func getTraceIDHeader(t *testing.T, evInfos []recordevents.EventInfo) string {
       ^
  3 issues:
  * unused: 3
```
@simkam
Copy link
Contributor Author

simkam commented Jan 26, 2026

/test reconciler-tests

Copy link
Member

@creydr creydr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Jan 26, 2026
@knative-prow
Copy link

knative-prow bot commented Jan 26, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: creydr, simkam

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 26, 2026
@knative-prow knative-prow bot merged commit ccf232a into knative:main Jan 26, 2026
40 checks passed
@simkam simkam deleted the linter-fix branch February 5, 2026 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. area/test-and-release Test infrastructure, tests or release lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants