-
Notifications
You must be signed in to change notification settings - Fork 110
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
deprecate unneeded utils.Testing #4350
Conversation
// Testing returns true when you are running in test suite. | ||
// Deprecated: this is in the standard library now. | ||
func Testing() bool { | ||
// TODO switch to official testing.Testing method when we are on go 1.21 | ||
return flag.Lookup("test.v") != nil | ||
return testing.Testing() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I doubt this is being used by external users. Seems like it is only used internally in utils/test_detector/main.go
Not sure how we go about deleting unused but public code. (we are pre v1 so maybe find to delete)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ideally do one release with it deprecated, next release removes? can be convinced to delete now if you think that's the move
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I doubt people are upgrading every single week to the new version. Unless we want to deprecate this now and remove in 1-2 months (or never remove it), I doubt it makes a difference if we wait a week to remove.
Safest is just to deprecate. But I would hope that we could just remove.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not too involved in rdk releases nowadays
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will leave as-is and make it a problem for future me
What changed
Why
This is in the standard library now