Skip to content

Conversation

@HarryCampion
Copy link
Collaborator

  • Update Makefile.presentation: Add install-marp target with global check and local install fallback to ${INSTALL_DIR}/node_modules/.bin/marp
  • Update Makefile.agentic: Add COPILOT_BIN variable with global check, update install-copilot to check global first, then local ${INSTALL_DIR}
  • Add comprehensive test suite (test_install_patterns.py) with 25 tests:
    • Mock tests for install behavior without side effects
    • Integration tests using make dry-run for CI pipelines
    • Pattern consistency tests across all Makefiles
  • Exception: gh CLI only checks global (no straightforward local install)

- Update Makefile.presentation: Add install-marp target with global check
  and local install fallback to ${INSTALL_DIR}/node_modules/.bin/marp
- Update Makefile.agentic: Add COPILOT_BIN variable with global check,
  update install-copilot to check global first, then local ${INSTALL_DIR}
- Add comprehensive test suite (test_install_patterns.py) with 25 tests:
  - Mock tests for install behavior without side effects
  - Integration tests using make dry-run for CI pipelines
  - Pattern consistency tests across all Makefiles
- Exception: gh CLI only checks global (no straightforward local install)
@HarryCampion HarryCampion requested a review from tschm January 10, 2026 18:02
@HarryCampion
Copy link
Collaborator Author

Standardize Install Patterns Across Makefiles

Summary

This PR standardizes the installation patterns across all Makefiles to follow a consistent approach:

  1. Check if globally installed (command -v <tool>)
  2. Check if locally installed in ${INSTALL_DIR} (defaults to ./bin)
  3. Install to local directory if neither exists

This ensures tools are installed in the project's ./bin directory rather than globally, providing better isolation and reproducibility across environments.

Changes

Makefile.presentation

  • Added new install-marp target following the standard pattern
  • Added MARP_BIN variable: checks global first → falls back to ${INSTALL_DIR}/node_modules/.bin/marp
  • Changed from npm install -g (global) to npm install --prefix "${INSTALL_DIR}" (local)
  • All presentation targets now depend on install-marp

Makefile.agentic

  • Added COPILOT_BIN variable: checks global first → falls back to ${INSTALL_DIR}/copilot
  • Updated install-copilot to check global installation before checking local
  • Updated all targets to use ${COPILOT_BIN} instead of hardcoded ${INSTALL_DIR}/copilot

Exceptions

  • gh CLI (Makefile.gh): Only checks for global installation and displays a warning if not found - there's no straightforward way to install it locally to ./bin

Tests Added

New test file tests/test_rhiza/test_install_patterns.py with 25 tests:

Test Class Description
TestInstallPatternConsistency Validates all install targets exist and follow the global-check-then-local pattern
TestMockInstallBehavior Mock-based tests for install behavior without side effects
TestIntegrationInstallPatterns Dry-run integration tests that work in CI pipelines
TestInstallDirVariable Tests for INSTALL_DIR variable behavior
TestInstallTargetDocumentation Validates all install targets have help comments

Install Pattern Reference

Tool Makefile Pattern
uv/uvx Makefile ✅ Global check → ${INSTALL_DIR} fallback
gh Makefile.gh ⚠️ Global check only (exception)
copilot Makefile.agentic ✅ Global check → ${INSTALL_DIR} fallback
marp Makefile.presentation ✅ Global check → ${INSTALL_DIR} fallback

Testing

# Run all install pattern tests
make test

# Or run just the install pattern tests
pytest tests/test_rhiza/test_install_patterns.py -v

All 94 tests pass (1 skipped).

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.

3 participants