-
Notifications
You must be signed in to change notification settings - Fork 23
Reuse go-common code and small refactoring #256
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
Conversation
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.
Pull request overview
This PR refactors the codebase to eliminate code duplication by reusing constants and methods from the go-common library instead of maintaining local copies. The refactoring removes the locally defined GetHostNamespacePath() function and namespace constants, replacing them with imports from github.com/harvester/go-common/common.
- Removes duplicate
GetHostNamespacePath()implementation frompkg/utils/process.go - Replaces local namespace constants (
HarvesterNS,LonghornSystemNamespaceName) with constants fromgo-common - Updates
prometheus/procfsdependency from v0.16.1 to v0.17.0
Reviewed changes
Copilot reviewed 10 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Adds replace directive for go-common fork and updates Go version and procfs dependency |
| go.sum | Updates dependency checksums for go-common fork and procfs upgrade |
| vendor/modules.txt | Reflects vendored dependency changes with updated versions |
| vendor/github.com/harvester/go-common/common/constants.go | Adds shared namespace constants |
| vendor/github.com/prometheus/procfs/proc_statm.go | New file from procfs v0.17.0 upgrade |
| vendor/github.com/prometheus/procfs/proc_stat.go | Adds StartCode, EndCode, StartStack fields from procfs upgrade |
| vendor/github.com/prometheus/procfs/meminfo.go | Adds new memory info fields from procfs upgrade |
| vendor/github.com/prometheus/procfs/mdstat.go | Adds reshape state support from procfs upgrade |
| vendor/github.com/prometheus/procfs/Makefile.common | Updates golangci-lint version and formatting rules |
| pkg/utils/utils.go | Replaces local GetHostNamespacePath calls with go-common version and removes constants |
| pkg/utils/process.go | Deleted file - functionality moved to go-common |
| pkg/lvm/lvm.go | Uses GetHostNamespacePath from go-common |
| pkg/block/block_device.go | Uses GetHostNamespacePath from go-common |
| pkg/controller/blockdevice/controller.go | Uses HarvesterNS constant from go-common |
| pkg/provisioner/lvm.go | Uses HarvesterNS constant from go-common |
| pkg/webhook/storageclass/validator.go | Uses HarvesterNS constant from go-common |
| pkg/webhook/blockdevice/validator.go | Uses LonghornSystemNamespaceName from go-common and extracts helper function |
| pkg/webhook/blockdevice/validator_test.go | Uses LonghornSystemNamespaceName constant from go-common |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
8912c49 to
09165c7
Compare
|
This pull request is now in conflict. Could you fix it @martindekov? 🙏 |
|
Please rebase and squash |
9be28b9 to
aafb874
Compare
|
rebased and squashed @Vicente-Cheng |
Reusing go-common code redefined in the repo including: * constants * methods * leftover feedback from previous review * fix deprecated namespaces * fix linter errors Signed-off-by: Martin Dekov <martin.dekov@suse.com>
aafb874 to
f38829a
Compare
w13915984028
left a comment
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.
|
@Vicente-Cheng when you are available if you can approve as well as you are code owner or anyone from the @harvester/storage would be appreciated! |
Vicente-Cheng
left a comment
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.
thanks for the enhancement
Reusing go-common code redefined in the repo including:
Problem:
harvester/harvester#9369
Repo re-defined methods and constants which were already defined in go-common.
Solution:
Reused the methods and constants from go-common as opposed to maintaining the duality.
Related Issue:
harvester/harvester#9369
Test plan:
N/A refactoring - all tests should pass