refactor: streamline makefile by consolidating commands and improving readability #1046
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the Makefile to improve readability and streamline command execution by replacing $(MAKE) calls with direct target dependencies, adding @ prefixes to suppress command echoing, and removing unused variables and version checks.
Changes:
- Consolidated make target invocations by replacing
$(MAKE) targetcalls with prerequisite dependencies - Added
@prefix to all commands for cleaner output during execution - Removed unused elements (BRANCH variable,
go version, andgolangci-lint --versioncalls)
| @rm -rf ./bin | ||
| @rm -rf /go/bin/terraform-provider-power-platform | ||
|
|
||
| deps: clean |
There was a problem hiding this comment.
The deps target now has a dependency on clean, which means every time deps is called, the test cache and binaries will be cleaned. Since build and install targets depend on deps, this causes unnecessary cleanup operations during normal builds. This could slow down the development workflow significantly. Consider removing the clean dependency from deps and only add it where explicitly needed (like in unittest, acctest, test, coverage targets).
| deps: clean | |
| deps: |
| build: deps | ||
| @go fmt ./... | ||
| @go build -o ./bin/ |
There was a problem hiding this comment.
The build target depends on deps, which now depends on clean. This means every build will clean the test cache and binaries first, defeating the purpose of incremental builds. The old makefile explicitly called $(MAKE) deps without a clean dependency, allowing faster iterative builds. Consider reverting deps to not depend on clean, and ensure clean is only called when explicitly needed (tests, coverage, precommit).
| install: deps | ||
| @go fmt ./... | ||
| @go install |
There was a problem hiding this comment.
The install target depends on deps, which now depends on clean. This creates a problematic behavior where running 'make install' during development will always clean the test cache and binaries, slowing down the workflow. The old makefile kept install fast by only calling deps without cleaning. Consider removing the clean dependency from deps to maintain efficient incremental builds.
|
|
||
| coverage: clean install | ||
| @echo "Changed files:" | ||
| @gh pr diff --name-only || true |
There was a problem hiding this comment.
The coverage target uses 'gh pr diff --name-only || true' which will silently fail if gh is not installed or if not in a PR context. While the '|| true' prevents the make command from failing, it would be more helpful to users to provide feedback when this command fails. Consider adding an explicit check or error message when gh command is unavailable.
| @gh pr diff --name-only || true | |
| @if command -v gh >/dev/null 2>&1; then \ | |
| gh pr diff --name-only || echo "Note: 'gh pr diff' failed (are you running in a PR context?); skipping changed-files list."; \ | |
| else \ | |
| echo "Note: GitHub CLI 'gh' not found; skipping changed-files list."; \ | |
| fi |
| lint: | ||
| @golangci-lint run | ||
|
|
||
| precommit: clean build lint unittest userdocs |
There was a problem hiding this comment.
The precommit target lists 'clean' as a dependency, but 'build' already depends on 'deps' which depends on 'clean', and 'unittest' also depends on 'clean'. This creates redundant dependencies. While Make will only execute clean once, the explicit listing makes the dependency chain unclear. Consider removing 'clean' from the precommit dependencies since it's already transitively included through build and unittest.
| precommit: clean build lint unittest userdocs | |
| precommit: build lint unittest userdocs |
No description provided.