-
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
[chore] Enable all (passing) goleak checks #30451
Conversation
It would be great to generate this test along with #27849. If we need to skip it for some packages we can add |
Would it be alright if I open an issue for this and add it as a work item to issue #30438? I have some questions about how that would work, and I'm thinking it would be good to keep a PR this big to just copy changes. I worry that introducing functionality changes would make it that much worse to review. Also, if these files look good to reviewers, they shouldn't change if being generated by |
Sounds good to me. 👍 We can do it as a follow-up |
I've filed #30483 for using |
Apologies for all of the updates, it looks like over the last few days a lot of packages have added opencensus as an indirect dependency, causing them to fail goleak checks. I'm just going to do this whole PR over again and post the removals in one fell swoop at this point, rather than updating one at a time. |
This should be ready to review again. I apologize for the size. For easy reviewing, you can view the output of the following command I ran against this branch to confirm that it's just the same thing copied in many places.
Here's each part explained:
|
**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.--> This PR contains 3 main changes: 1. Make a copy of the same file into every package whose `goleak` check is currently passing. The only change for the `package_test.go` file in each package is the package name itself. 2. Rename `processor/tailsamplingprocessor/main_test.go` to `package_test.go` for uniformity. `package_test.go` is the name we chose for core (see discussion [here](open-telemetry/opentelemetry-collector#9173 (comment))). 3. Run `make gotidy` to add `goleak` dependency. I included the script used to make these changes [in the issue](open-telemetry#30438 (comment)). **Link to tracking Issue:** <Issue number if applicable> open-telemetry#30438
**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.--> This PR contains 3 main changes: 1. Make a copy of the same file into every package whose `goleak` check is currently passing. The only change for the `package_test.go` file in each package is the package name itself. 2. Rename `processor/tailsamplingprocessor/main_test.go` to `package_test.go` for uniformity. `package_test.go` is the name we chose for core (see discussion [here](open-telemetry/opentelemetry-collector#9173 (comment))). 3. Run `make gotidy` to add `goleak` dependency. I included the script used to make these changes [in the issue](open-telemetry#30438 (comment)). **Link to tracking Issue:** <Issue number if applicable> open-telemetry#30438
Description:
This PR contains 3 main changes:
goleak
check is currently passing. The only change for thepackage_test.go
file in each package is the package name itself.processor/tailsamplingprocessor/main_test.go
topackage_test.go
for uniformity.package_test.go
is the name we chose for core (see discussion here).make gotidy
to addgoleak
dependency.I included the script used to make these changes in the issue.
Link to tracking Issue:
#30438
Testing:
Ran tests in every referenced package, they're passing.