-
Notifications
You must be signed in to change notification settings - Fork 325
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
use FS struct for proc and sys mount points in submodules #149
use FS struct for proc and sys mount points in submodules #149
Conversation
One drawback of this approach is that the affected subpackages now have a dependency on the main procfs and/or sysfs packages. It might be better to add the FS type to the internal/util package and have the sub-modules use that instead of the FS from procfs and sysfs. |
889877b
to
5f92a6a
Compare
Signed-off-by: Paul Gier <pgier@redhat.com>
Signed-off-by: Paul Gier <pgier@redhat.com>
Signed-off-by: Paul Gier <pgier@redhat.com>
Moves the FS type definition to the internal/util package. This allows the sub-packages bcache, blockdevice, nfs, and xfs to work without bringing in procfs and sysfs as dependencies. Signed-off-by: Paul Gier <pgier@redhat.com>
5f92a6a
to
d92cb22
Compare
@mdlayher @discordianfish PTAL. I've updated this PR to move the FS/string type to the internal/util package. This allows the sub-packages bcache, blockdevice, nfs, and xfs to work without a dependency on the main procfs and sysfs packages, which was the original purpose of #136. And I made changes to the procfs and sysfs packages to be consistent with the subpackages. |
Great, I like that! |
Avoids the anti-pattern of using a 'util' package. Signed-off-by: Paul Gier <pgier@redhat.com>
Signed-off-by: Paul Gier <pgier@redhat.com>
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, I like it.
@mdlayher Any other issues? |
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 thanks!
@discordianfish everything look good to you? |
Great, thanks! :) |
Update the Linux provider to use procfs.NewFS, and stop using the procfs.FS.Path method which was removed in prometheus/procfs#149. The changed code works with both the old and new procfs APIs.
…#149) * use FS struct for proc and sys mount points in submodules Signed-off-by: Paul Gier <pgier@redhat.com> * xfs: fix staticcheck issue Signed-off-by: Paul Gier <pgier@redhat.com> * better naming for filesystem handles in submodules Signed-off-by: Paul Gier <pgier@redhat.com> * refactor FS to reduce inter-package dependencies Moves the FS type definition to the internal/util package. This allows the sub-packages bcache, blockdevice, nfs, and xfs to work without bringing in procfs and sysfs as dependencies. Signed-off-by: Paul Gier <pgier@redhat.com> * move fs.go to a new internal.fs package Avoids the anti-pattern of using a 'util' package. Signed-off-by: Paul Gier <pgier@redhat.com> * move default proc and sys mount point constants to internal/fs Signed-off-by: Paul Gier <pgier@redhat.com>
Update docs and CI for MSRV
Based on discussion in #148, instead of removing the FS type completely, this PR moves the FS type to a shared internal package and exposes a more consistent API for each of the subpackages.