Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[chore][golangci-lint] Remove gosec excludes #10411

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

crobert-1
Copy link
Member

@crobert-1 crobert-1 commented Jun 14, 2024

Description

The upstream issue was fixed by golangci/golangci-lint#4748, which was included in release v1.59.0 of golangci-lint. From local testing, we're pulling version v1.59.0 of golangci-lint, so the issue should be resolved.

Local runtime with excludes:

$ .tools/golangci-lint run -v --enable-only gosec
...
INFO Execution took 1.866075148s
INFO Execution took 1.218805785s
INFO Execution took 1.09527985s

Local runtime without excludes:

$ .tools/golangci-lint run -v --enable-only gosec
...
INFO Execution took 2.244716429s
INFO Execution took 1.539717296s
INFO Execution took 1.530163777s

Note: I ran .tools/golangci-lint cache clean between each test to clean the cache and keep results as consistent as possible.

Link to tracking issue

Fixes #10213

@crobert-1 crobert-1 requested review from a team and atoulme June 14, 2024 18:05
Copy link

codecov bot commented Jun 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.40%. Comparing base (a2289fd) to head (30c4ee6).
Report is 26 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10411      +/-   ##
==========================================
- Coverage   92.52%   92.40%   -0.13%     
==========================================
  Files         388      387       -1     
  Lines       18310    18323      +13     
==========================================
- Hits        16942    16931      -11     
- Misses       1022     1046      +24     
  Partials      346      346              

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

codeboten pushed a commit to open-telemetry/opentelemetry-collector-contrib that referenced this pull request Jun 14, 2024
**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
The upstream issue was fixed by
golangci/golangci-lint#4748, which was included
in release
[`v1.59.0`](https://github.com/golangci/golangci-lint/releases/tag/v1.59.0)
of `golangci-lint`. From local testing, we're pulling version `v1.59.1`
of `golangci-lint`, so the issue should be resolved.

Local runtime with excludes:
```
$ .tools/golangci-lint run -v --enable-only gosec
...
INFO Execution took 10.927544867s
INFO Execution took 8.011302204s
INFO Execution took 7.716441258s
INFO Execution took 7.441336833s
```
Local runtime without excludes:
```
$ .tools/golangci-lint run -v --enable-only gosec
...
INFO Execution took 9.780250262s
INFO Execution took 8.175492516s
INFO Execution took 7.550060974s
INFO Execution took 7.526585686s
```

Note: I ran `.tools/golangci-lint cache clean` between each test to
clean the cache and keep results as consistent as possible. I admit that
I don't know why the values keep going down with every run, the cache
cleaning command may not entirely be working.

**Link to tracking Issue:** <Issue number if applicable>
These excludes were introduced in
#33192
I've opened a PR in core for this issue as well:
open-telemetry/opentelemetry-collector#10411
@codeboten codeboten merged commit 48af1b8 into open-telemetry:main Jun 14, 2024
49 checks passed
@github-actions github-actions bot added this to the next release milestone Jun 14, 2024
@crobert-1 crobert-1 deleted the remove_gosec_excludes branch June 15, 2024 14:23
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove G601 G113 gosec excludes
2 participants