-
Notifications
You must be signed in to change notification settings - Fork 24
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
feat(region): more debug logs #1007
Conversation
if err != nil { | ||
debug.Printf("[Regions] Fail to open the cache: %v\n", err) | ||
} else { |
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.
We are more used to check err != nil
in Go. So I'm refactoring to use this pattern here.
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.
Question: Why don't we use defer fd.Close()?
It seems more secure IMO.
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 don't know. Maybe it's useless in this case to wait longer before closing the file descriptor: we decode its content then immediately close it.
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 Then 👌
if err != nil { | ||
debug.Printf("[Regions] Failed to save the cache: %v\n", err) | ||
return regionsCache, nil | ||
} |
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.
We are more used to check err != nil
in Go. So I'm refactoring to use this pattern here.
if err != nil { | ||
debug.Printf("[Regions] Fail to open the cache: %v\n", err) | ||
} else { |
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.
Question: Why don't we use defer fd.Close()?
It seems more secure IMO.
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
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.
🚀
Fix #866
This may help debug #809.