feat: add prepare-dev in Makefile#1426
Conversation
Signed-off-by: Tom <yusencao@outlook.com>
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new prepare-dev Makefile target to automate local environment setup for development, addressing cgo-related package config and lint preparation.
- Introduces
prepare-devphony target - Adjusts pkg-config prefixes under
mk/ - Hooks in lint setup and environment bootstrap scripts
Comments suppressed due to low confidence (1)
Makefile:142
- [nitpick] Consider renaming
prepare-devto something likedev-setupor adding a brief comment above the target to clarify its intent in the Makefile help section.
prepare-dev:
|
|
||
| .PHONY: prepare-dev | ||
| prepare-dev: | ||
| $(QUIET) find $(ROOT_DIR)/mk -name "*.pc" | xargs sed -i "s#^prefix=.*#prefix=${ROOT_DIR}#g" |
There was a problem hiding this comment.
Using sed -i without a backup suffix is not portable on macOS/BSD. Consider using sed -i.bak or detecting the OS to pass the appropriate -i '' flag for compatibility.
| $(QUIET) find $(ROOT_DIR)/mk -name "*.pc" | xargs sed -i "s#^prefix=.*#prefix=${ROOT_DIR}#g" | |
| $(QUIET) find $(ROOT_DIR)/mk -name "*.pc" | xargs $(SED_INPLACE) "s#^prefix=.*#prefix=${ROOT_DIR}#g" |
| .PHONY: prepare-dev | ||
| prepare-dev: | ||
| $(QUIET) find $(ROOT_DIR)/mk -name "*.pc" | xargs sed -i "s#^prefix=.*#prefix=${ROOT_DIR}#g" | ||
| hack/golangci-lint-prepare.sh |
There was a problem hiding this comment.
It may help to prefix this invocation with set -e or use || exit 1 so that failures in the lint prep script cause the Make target to fail fast.
| hack/golangci-lint-prepare.sh | |
| hack/golangci-lint-prepare.sh || exit 1 |
| prepare-dev: | ||
| $(QUIET) find $(ROOT_DIR)/mk -name "*.pc" | xargs sed -i "s#^prefix=.*#prefix=${ROOT_DIR}#g" | ||
| hack/golangci-lint-prepare.sh | ||
| bash -c "source ./kmesh_compile_env_pre.sh && dependency_pkg_install" |
There was a problem hiding this comment.
Sourcing via bash -c spawns a subshell, so environment changes won't persist. Consider sourcing directly (. ./kmesh_compile_env_pre.sh) in the current shell or wrapping both steps in a script.
| bash -c "source ./kmesh_compile_env_pre.sh && dependency_pkg_install" | |
| . ./kmesh_compile_env_pre.sh && dependency_pkg_install |
| gen-kmeshctl-doc: | ||
| hack/gen-kmeshctl-doc.sh | ||
|
|
||
| .PHONY: prepare-dev |
There was a problem hiding this comment.
[nitpick] This new target should be added to the Makefile's help output or documented in the README so contributors know when and how to run it.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Signed-off-by: Tom <yusencao@outlook.com>
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hzxuzhonghu The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind enhancement
What this PR does / why we need it:
Local development environment is confusing because of
cgo-related code and local environment setting-up documents should be added.Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Does this PR introduce a user-facing change?: