Skip to content

Conversation

crivetimihai
Copy link
Member

@crivetimihai crivetimihai commented Jul 27, 2025

Closes #507

🚨 Critical Issues

  • 1. Missing .PHONY Declarations - I can see lines 622-625 already include most targets including print-runtime, print-image, container-validate-env, container-check-ports, container-wait-healthy
  • 2. Undefined Target Referenced - container-wait-healthy exists at line 852
  • 11. Wrong Dependency Name in container-validate - Line 823 correctly uses container-check-ports

⚠️ Important Issues

  • 3. Platform-Specific Commands Without Checks - Both issues fixed:
    • Line 835: lsof check exists with proper handling
    • Line 732: systemctl check exists with proper handling
  • 4. IBM Cloud Target Missing File Check - Line 1267: ibmcloud-check-env has the check @test -f .env.ce || {...}
  • 5. Clean Target Missing Files
  • 6. Inconsistent xargs Usage - Still has bare -r in devpi-stop
  • 12. Local PyPI Port/URL Mismatch - Fixed to consistently use port 8085
  • 13. Orphaned .PHONY Marker - N/A (no orphaned marker found)
  • 14. shfmt PATH Issue - Fixed with SHFMT variable and multiple location checks
  • 15. Hard-coded --platform=linux/amd64 - Fixed with auto-detection at line 631
  • 16. docker buildx Builder Never Re-used - Fixed at line 798 with existence check
  • 17. Conflicting Flags in compose-restart - Fixed at line 1095 - now separate pull and build commands

📝 Minor Issues

  • 7. Static Container File - Line 614 dynamically detects: CONTAINER_FILE ?= $(shell [ -f "Containerfile" ] && echo "Containerfile" || echo "Dockerfile")
  • 8. Test File Assumption - Line 314: pytest-examples has the check for test_readme.py
  • 9. Shell Script Finding - Line 2933: SHELL_SCRIPTS excludes all the directories
  • 10. Unused Variable - SED_INPLACE still defined but unused
  • 18. Non-Persistent Error Handling - Line 179: clean target uses bash -eu -o pipefail -c
  • 19. Duplicate .PHONY Declarations - Actually no duplicate found - YAML/JSON/TOML linters are only in LINTERS list, not separate .PHONY

Signed-off-by: Mihai Criveti <[email protected]>
Signed-off-by: Mihai Criveti <[email protected]>
Signed-off-by: Mihai Criveti <[email protected]>
Signed-off-by: Mihai Criveti <[email protected]>
@crivetimihai crivetimihai merged commit 9340cbf into main Jul 27, 2025
19 of 21 checks passed
@crivetimihai crivetimihai deleted the 507-makefile-fixes branch July 27, 2025 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Makefile missing .PHONY declarations and other issues
1 participant