-
Notifications
You must be signed in to change notification settings - Fork 168
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
refact(event): refactor event manager #324
Conversation
Can one of the admins verify this patch? |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Assign the PR to them by writing The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these OWNERS Files:
You can indicate your approval by writing |
dc0d9d3
to
0d3cc59
Compare
store/event.go
Outdated
intervalNum = retryThreshold | ||
} | ||
|
||
event.InTime = event.InTime.Add(time.Duration(time.Duration(intervalNum) * retryInterval)) |
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.
event.InTime = event.InTime.Add(intervalNum * retryInterval)
retryInterval is already a Duration.
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.
Still need one conversion: event.InTime = event.InTime.Add(time.Duration(intervalNum) * retryInterval)
The outer conversion is not necessary.
store/event.go
Outdated
// outTimeout represents the timeout for event to keep "out" status. | ||
// After the event is out, then its status will be "handling" after it starts to be handled, | ||
// otherwise, it will be re-enqueued and its status will change back to "in". | ||
outTimeout, _ = time.ParseDuration("-3m") |
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.
Is there any reason for you to use time.ParseDuration? It seems that you exactly know the timeout duration.
outTimeout = -3 * time.Minute
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.
Fixed
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
Fixes #302
Changes