-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
gui: /rest/system/browse with no arguments returns drives on Windows … #3203
Conversation
kernel32, err := syscall.LoadDLL("kernel32.dll") | ||
if err != nil { | ||
return drives | ||
} |
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.
Why not just return 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.
doesn't really matter does 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.
It matters only in that when looking at this I need to keep track of if we've actually done anything with drives
to see what we return in the error cases. That's trivial in the first case but when looking at line 34 I actually have to parse lines 18-33 as well to understand what it means.
As an aside, shouldn't we just return ([]string, error)
and let the API return a 500 when this fails on Windows? The empty success response isn't meaningful by itself, is it?
Bump. |
@st-review merge it! |
👌 Merged as 1612bac. Thanks, @AudriusButkevicius! |
See title