Skip to content

refactor: streamline makefile by consolidating commands and improving readability #1046

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.

Sign up for GitHub

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

Open
polatengin wants to merge 1 commit into
base: main
Choose a base branch
from
+51 −0
Open

refactor: streamline makefile by consolidating commands and improving readability #1046

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
clean:
@go clean -testcache
@rm -rf ./bin
@rm -rf /go/bin/terraform-provider-power-platform

deps: clean
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

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).

Suggested change
deps: clean
deps:

Copilot uses AI. Check for mistakes.
@go mod tidy

build: deps
@go fmt ./...
@go build -o ./bin/
Comment on lines +9 to +11
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

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).

Copilot uses AI. Check for mistakes.

install: deps
@go fmt ./...
@go install
Comment on lines +13 to +15
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.

userdocs:
@go generate
@tfplugindocs validate --provider-name powerplatform

userstory:
@./scripts/user_story_prompt.sh

unittest: clean install
@TF_ACC=0 go test -p 16 -timeout 10m -v -cover ./... -run "^TestUnit$(TEST)"

acctest: clean install
ifeq ($(USE_PROXY),1)
@HTTP_PROXY=http://127.0.0.1:8080 HTTPS_PROXY=http://127.0.0.1:8080 TF_ACC=1 go test -p 10 -timeout 300m -v ./... -run "^TestAcc$(TEST)"
else
@TF_ACC=1 go test -p 10 -timeout 300m -v ./... -run "^TestAcc$(TEST)"
endif

test: clean install
@TF_ACC=1 go test -p 10 -timeout 300m -v -cover ./...

coverage: clean install
@echo "Changed files:"
@gh pr diff --name-only || true
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
@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

Copilot uses AI. Check for mistakes.
@echo "Running tests"
@TF_ACC=0 go test -p 16 -timeout 10m -v -cover -coverprofile=test-coverage.out ./... -run "^TestUnit$(TEST)"
@echo "Generating coverage report"
@go tool cover -func=test-coverage.out

netdump:
@mitmdump -p 8080 -w /tmp/mitmproxy.dump

lint:
@golangci-lint run

precommit: clean build lint unittest userdocs
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
precommit: clean build lint unittest userdocs
precommit: build lint unittest userdocs

Copilot uses AI. Check for mistakes.