-
Notifications
You must be signed in to change notification settings - Fork 147
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
Make maxSleep and maxRetires configurable when building options #94
Make maxSleep and maxRetires configurable when building options #94
Conversation
Make goleak options more flexible to adapt to users' variety scenarios Signed-off-by: Kante Yin <kerthcet@gmail.com>
cc @prashantv for review maybe? |
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.
Thank you for the contribution. Left a few comments to improve the PR.
Signed-off-by: Kante Yin <kerthcet@gmail.com>
@sywhang PTAL |
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.
This looks good to me. It would be useful to have in a release soon together with the x/lint go.mod change because then I can simplify my Kubernetes PR: https://github.com/kubernetes/kubernetes/pull/115456/files#r1097270472
Thanks @pohly , this seems need the privilege of maintainer to run the workflow. cc @sywhang @prashantv |
Codecov Report
@@ Coverage Diff @@
## master #94 +/- ##
==========================================
+ Coverage 94.92% 96.52% +1.59%
==========================================
Files 5 5
Lines 138 230 +92
==========================================
+ Hits 131 222 +91
- Misses 4 5 +1
Partials 3 3
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
LGTM with minor comments.
CC @sywhang
Co-authored-by: Abhinav Gupta <mail@abhinavg.net>
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.
LGTM. Thanks for the contribution
Sorry for the delay, as mentioned on the issue, I'm not sure we should be exposing these options, but try to capture a single option to configure retries (e.g., RetryTime). It seems like an implementation leak to expose the underlying num retries / max sleep etc. |
…ns (uber-go#94)" This reverts commit 751da59.
Make goleak options more flexible to adapt to users' variety scenarios
Signed-off-by: Kante Yin kerthcet@gmail.com
fix #93