-
Notifications
You must be signed in to change notification settings - Fork 54
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
🐛 main.go: improve logging for configuration of global auth #1316
🐛 main.go: improve logging for configuration of global auth #1316
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1316 +/- ##
==========================================
+ Coverage 76.07% 76.09% +0.02%
==========================================
Files 40 40
Lines 2378 2380 +2
==========================================
+ Hits 1809 1811 +2
Misses 401 401
Partials 168 168
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
cmd/manager/main.go
Outdated
return "" | ||
} | ||
if err != nil { | ||
logger.Error(err, "could not stat auth file path", "path", authFilePath) | ||
logger.Error(err, "unable to stat auth file path", "path", authFilePath) |
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.
unable to stat auth file path
is kind of meaning less in a way. How would the user know that we are talking about os.stat and what does that mean.
/hold as the existing lgtm would lead to merging of the PR and I have a review comment #1316 (comment) which should be addressed. |
Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
0513d61
cee5d55
to
0513d61
Compare
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
/hold cancel |
360f892
Description
Reviewer Checklist