Skip to content
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

Feature/remove disk partition args on bsds #975

Merged
merged 3 commits into from
Oct 24, 2020

Conversation

shirou
Copy link
Owner

@shirou shirou commented Oct 20, 2020

I found psutil ignore argument on BSD systems according to the issue.

This PR changes to return all partition information.

(These code is written in 2016 and I can not remember why add path evaluation)

According to giampaolo/psutil#906,
all BSD system returns same information `df` and `df -a`.
@shirou shirou force-pushed the feature/remove_disk_partition_args_on_bsds branch 3 times, most recently from caefcbb to 5600b03 Compare October 21, 2020 13:17
@shirou shirou force-pushed the feature/remove_disk_partition_args_on_bsds branch from 5600b03 to 23a5246 Compare October 21, 2020 13:20
Copy link
Collaborator

@Lomanic Lomanic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the Pull request labeler action is a good idea. I would add - **/*_bsd.go for os:freebsd and os:openbsd automatic tagging to not miss these files:

find -name '*_bsd*.go' -exec grep "^// +build" {} +
./load/load_bsd.go:// +build freebsd openbsd
./process/process_bsd.go:// +build darwin freebsd openbsd
./host/host_bsd.go:// +build darwin freebsd openbsd

@@ -14,6 +13,8 @@ func Partitions(all bool) ([]PartitionStat, error) {
return PartitionsWithContext(context.Background(), all)
}

// PartitionsWithContext returns disk partition.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to have the same documentation centralized for all OSes (like in psutil for example), as godoc.org only displays documentation for linux/amd64. So this should be moved to disk/disk_linux.go (saying "all argument is ignored on darwin, freebsd and openbsd, see: giampaolo/psutil#906"). This documentation will be moved to disk/disk.go once Partitions() definition is moved there (#761).

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, agree. sorry, that comment is a my tentative note and forget to remove. I will open another PR which aligns to #761. At that PR, I will move comment to disk_linux.go

@shirou
Copy link
Owner Author

shirou commented Oct 22, 2020

Thank you for the comment! I will add _bsd to labbeler.

@shirou shirou merged commit cf222ab into master Oct 24, 2020
@shirou shirou deleted the feature/remove_disk_partition_args_on_bsds branch October 24, 2020 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants