-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Return all Windows partitions #1347
Conversation
@shirou any chance you'd be available to review? |
Thank you for contribution(and sorry to late response)! Is the purpose of this PR to get all partition information in case of errors?
|
Thanks for your reply!
|
Thank you. You are correct that gopsutil is minimalist oriented. Therefore, I am happy not to use these libraries. However, if you could align the method names, etc. with these libraries (ex. |
I remember there already is a multierr-like thing in gopsutil though, maybe reuse or adapt that? https://github.com/shirou/gopsutil/blob/16b3aac6adb5ef7c2c30e1b1cce90c35069b829a/host/types.go |
Oh, Great! So, could you move this struct and functions to under |
I found errors.Join() is now added to |
Please take a look. I have refactored Warnings to be part of internal/common. |
@shirou please let me know? |
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! Thank you so much!
For changes like these that break compatibility may I recommend you add |
First PR here. I had the assumption that given that this was an internal package, it wasn’t imported directly downstream. Sorry for the breakage. |
I have opened #1379 to fix. (type alias is better, thank you for your suggestion.) |
Return all partitions on Windows and all errors rather than returning early.