Skip to content
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

Mark VerifyNone as a test Helper #75

Merged
merged 1 commit into from Aug 30, 2022
Merged

Conversation

tallclair
Copy link
Contributor

Mark VerifyNone(...) as a test Helper function, so that it reports the error at the call site, rather than leaks.go:78:.

From the go docs:

Helper marks the calling function as a test helper function. When printing file and line information, that function will be skipped. Helper may be called simultaneously from multiple goroutines.

This is useful for tests that verify go routines at multiple points, so failures can be more easily distinguished.

@CLAassistant
Copy link

CLAassistant commented Aug 17, 2022

CLA assistant check
All committers have signed the CLA.

Copy link
Collaborator

@abhinav abhinav left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, @tallclair!
This is a reasonable request, but we'd prefer to do this in a backwards compatible way.
I've left a suggestion on how we can do that.

Separately, can you please also sign the CLA? We cannot land the change without that.

leaks.go Outdated Show resolved Hide resolved
@tallclair
Copy link
Contributor Author

I'm waiting on approval from my org to sign the CLA, sorry I may have jumped the gun on this.

@tallclair
Copy link
Contributor Author

Sorry for the delay. CLA is signed, and comments have been addressed.

@abhinav abhinav merged commit 2dfebe8 into uber-go:master Aug 30, 2022
@abhinav
Copy link
Collaborator

abhinav commented Aug 30, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants