ML-187: restructure pre-commit checks to run conditionally
Gate each check behind staged file pattern matching so a docs or backlog change no longer triggers the full test suite. Add presto pytest and Docker image validation guards. Fall back to running all checks when invoked directly (not via git commit).
This commit is contained in:
Claudio Ortolina
@@ -29,13 +29,15 @@ Before exploring the codebase for any task, read `docs/architecture.md`. Do this
|
||||
|
||||
<!-- usage-rules-start -->
|
||||
<!-- usage_rules-start -->
|
||||
|
||||
## usage_rules usage
|
||||
|
||||
_A config-driven dev tool for Elixir projects to manage AGENTS.md files and agent skills from dependencies_
|
||||
|
||||
## Using Usage Rules
|
||||
|
||||
Many packages have usage rules, which you should *thoroughly* consult before taking any
|
||||
action. These usage rules contain guidelines and rules *directly from the package authors*.
|
||||
Many packages have usage rules, which you should _thoroughly_ consult before taking any
|
||||
action. These usage rules contain guidelines and rules _directly from the package authors_.
|
||||
They are your best source of knowledge for making decisions.
|
||||
|
||||
## Modules & functions in the current app and dependencies
|
||||
@@ -54,7 +56,6 @@ mix usage_rules.docs Enum.zip
|
||||
mix usage_rules.docs Enum.zip/1
|
||||
```
|
||||
|
||||
|
||||
## Searching Documentation
|
||||
|
||||
You should also consult the documentation of any tools you are using, early and often. The best
|
||||
@@ -75,23 +76,27 @@ mix usage_rules.search_docs "making requests" -p req
|
||||
mix usage_rules.search_docs "Enum.zip" --query-by title
|
||||
```
|
||||
|
||||
|
||||
<!-- usage_rules-end -->
|
||||
<!-- usage_rules:elixir-start -->
|
||||
|
||||
## usage_rules:elixir usage
|
||||
|
||||
# Elixir Core Usage Rules
|
||||
|
||||
## Pattern Matching
|
||||
|
||||
- Use pattern matching over conditional logic when possible
|
||||
- Prefer to match on function heads instead of using `if`/`else` or `case` in function bodies
|
||||
- `%{}` matches ANY map, not just empty maps. Use `map_size(map) == 0` guard to check for truly empty maps
|
||||
|
||||
## Error Handling
|
||||
|
||||
- Use `{:ok, result}` and `{:error, reason}` tuples for operations that can fail
|
||||
- Avoid raising exceptions for control flow
|
||||
- Use `with` for chaining operations that return `{:ok, _}` or `{:error, _}`
|
||||
|
||||
## Common Mistakes to Avoid
|
||||
|
||||
- Elixir has no `return` statement, nor early returns. The last expression in a block is always returned.
|
||||
- Don't use `Enum` functions on large collections when `Stream` is more appropriate
|
||||
- Avoid nested `case` statements - refactor to a single `case`, `with` or separate functions
|
||||
@@ -104,6 +109,7 @@ mix usage_rules.search_docs "Enum.zip" --query-by title
|
||||
- There are many useful standard library functions, prefer to use them where possible
|
||||
|
||||
## Function Design
|
||||
|
||||
- Use guard clauses: `when is_binary(name) and byte_size(name) > 0`
|
||||
- Prefer multiple function clauses over complex conditional logic
|
||||
- Name functions descriptively: `calculate_total_price/2` not `calc/2`
|
||||
@@ -111,6 +117,7 @@ mix usage_rules.search_docs "Enum.zip" --query-by title
|
||||
- Names like `is_thing` should be reserved for guards
|
||||
|
||||
## Data Structures
|
||||
|
||||
- Use structs over maps when the shape is known: `defstruct [:name, :age]`
|
||||
- Prefer keyword lists for options: `[timeout: 5000, retries: 3]`
|
||||
- Use maps for dynamic key-value data
|
||||
@@ -123,6 +130,7 @@ mix usage_rules.search_docs "Enum.zip" --query-by title
|
||||
- Read the docs and options fully before using tasks
|
||||
|
||||
## Testing
|
||||
|
||||
- Run tests in a specific file with `mix test test/my_test.exs` and a specific test with the line number `mix test path/to/test.exs:123`
|
||||
- Limit the number of failed tests with `mix test --max-failures n`
|
||||
- Use `@tag` to tag specific tests, and `mix test --only tag` to run only those tests
|
||||
@@ -135,26 +143,32 @@ mix usage_rules.search_docs "Enum.zip" --query-by title
|
||||
|
||||
<!-- usage_rules:elixir-end -->
|
||||
<!-- usage_rules:otp-start -->
|
||||
|
||||
## usage_rules:otp usage
|
||||
|
||||
# OTP Usage Rules
|
||||
|
||||
## GenServer Best Practices
|
||||
|
||||
- Keep state simple and serializable
|
||||
- Handle all expected messages explicitly
|
||||
- Use `handle_continue/2` for post-init work
|
||||
- Implement proper cleanup in `terminate/2` when necessary
|
||||
|
||||
## Process Communication
|
||||
|
||||
- Use `GenServer.call/3` for synchronous requests expecting replies
|
||||
- Use `GenServer.cast/2` for fire-and-forget messages.
|
||||
- When in doubt, use `call` over `cast`, to ensure back-pressure
|
||||
- Set appropriate timeouts for `call/3` operations
|
||||
|
||||
## Fault Tolerance
|
||||
|
||||
- Set up processes such that they can handle crashing and being restarted by supervisors
|
||||
- Use `:max_restarts` and `:max_seconds` to prevent restart loops
|
||||
|
||||
## Task and Async
|
||||
|
||||
- Use `Task.Supervisor` for better fault tolerance
|
||||
- Handle task failures with `Task.yield/2` or `Task.shutdown/2`
|
||||
- Set appropriate task timeouts
|
||||
@@ -162,13 +176,15 @@ mix usage_rules.search_docs "Enum.zip" --query-by title
|
||||
|
||||
<!-- usage_rules:otp-end -->
|
||||
<!-- usage_rules-start -->
|
||||
|
||||
## usage_rules usage
|
||||
|
||||
_A config-driven dev tool for Elixir projects to manage AGENTS.md files and agent skills from dependencies_
|
||||
|
||||
## Using Usage Rules
|
||||
|
||||
Many packages have usage rules, which you should *thoroughly* consult before taking any
|
||||
action. These usage rules contain guidelines and rules *directly from the package authors*.
|
||||
Many packages have usage rules, which you should _thoroughly_ consult before taking any
|
||||
action. These usage rules contain guidelines and rules _directly from the package authors_.
|
||||
They are your best source of knowledge for making decisions.
|
||||
|
||||
## Modules & functions in the current app and dependencies
|
||||
@@ -187,7 +203,6 @@ mix usage_rules.docs Enum.zip
|
||||
mix usage_rules.docs Enum.zip/1
|
||||
```
|
||||
|
||||
|
||||
## Searching Documentation
|
||||
|
||||
You should also consult the documentation of any tools you are using, early and often. The best
|
||||
@@ -208,23 +223,27 @@ mix usage_rules.search_docs "making requests" -p req
|
||||
mix usage_rules.search_docs "Enum.zip" --query-by title
|
||||
```
|
||||
|
||||
|
||||
<!-- usage_rules-end -->
|
||||
<!-- usage_rules:elixir-start -->
|
||||
|
||||
## usage_rules:elixir usage
|
||||
|
||||
# Elixir Core Usage Rules
|
||||
|
||||
## Pattern Matching
|
||||
|
||||
- Use pattern matching over conditional logic when possible
|
||||
- Prefer to match on function heads instead of using `if`/`else` or `case` in function bodies
|
||||
- `%{}` matches ANY map, not just empty maps. Use `map_size(map) == 0` guard to check for truly empty maps
|
||||
|
||||
## Error Handling
|
||||
|
||||
- Use `{:ok, result}` and `{:error, reason}` tuples for operations that can fail
|
||||
- Avoid raising exceptions for control flow
|
||||
- Use `with` for chaining operations that return `{:ok, _}` or `{:error, _}`
|
||||
|
||||
## Common Mistakes to Avoid
|
||||
|
||||
- Elixir has no `return` statement, nor early returns. The last expression in a block is always returned.
|
||||
- Don't use `Enum` functions on large collections when `Stream` is more appropriate
|
||||
- Avoid nested `case` statements - refactor to a single `case`, `with` or separate functions
|
||||
@@ -237,6 +256,7 @@ mix usage_rules.search_docs "Enum.zip" --query-by title
|
||||
- There are many useful standard library functions, prefer to use them where possible
|
||||
|
||||
## Function Design
|
||||
|
||||
- Use guard clauses: `when is_binary(name) and byte_size(name) > 0`
|
||||
- Prefer multiple function clauses over complex conditional logic
|
||||
- Name functions descriptively: `calculate_total_price/2` not `calc/2`
|
||||
@@ -244,6 +264,7 @@ mix usage_rules.search_docs "Enum.zip" --query-by title
|
||||
- Names like `is_thing` should be reserved for guards
|
||||
|
||||
## Data Structures
|
||||
|
||||
- Use structs over maps when the shape is known: `defstruct [:name, :age]`
|
||||
- Prefer keyword lists for options: `[timeout: 5000, retries: 3]`
|
||||
- Use maps for dynamic key-value data
|
||||
@@ -256,6 +277,7 @@ mix usage_rules.search_docs "Enum.zip" --query-by title
|
||||
- Read the docs and options fully before using tasks
|
||||
|
||||
## Testing
|
||||
|
||||
- Run tests in a specific file with `mix test test/my_test.exs` and a specific test with the line number `mix test path/to/test.exs:123`
|
||||
- Limit the number of failed tests with `mix test --max-failures n`
|
||||
- Use `@tag` to tag specific tests, and `mix test --only tag` to run only those tests
|
||||
@@ -268,10 +290,13 @@ mix usage_rules.search_docs "Enum.zip" --query-by title
|
||||
|
||||
<!-- usage_rules:elixir-end -->
|
||||
<!-- mdex-start -->
|
||||
|
||||
## mdex usage
|
||||
|
||||
_Fast and extensible Markdown for Elixir_
|
||||
|
||||
@deps/mdex/usage-rules.md
|
||||
|
||||
<!-- mdex-end -->
|
||||
<!-- usage-rules-end -->
|
||||
|
||||
@@ -293,6 +318,7 @@ This project uses Backlog.md MCP for all task and project management activities.
|
||||
- **When to read it**: BEFORE creating tasks, or when you're unsure whether to track work
|
||||
|
||||
These guides cover:
|
||||
|
||||
- Decision framework for when to create tasks
|
||||
- Search-first workflow to avoid duplicates
|
||||
- Links to detailed guides for task creation, execution, and finalization
|
||||
|
||||
@@ -50,7 +50,7 @@ Run pi tests
|
||||
|
||||
- **Usage**: `dev:precommit`
|
||||
|
||||
Run checks before a commit
|
||||
Run checks before a commit (conditional on staged file types)
|
||||
|
||||
## `dev:readme`
|
||||
|
||||
|
||||
@@ -16,6 +16,24 @@ Key rules: imperative present tense, single-line under 60 characters, task ID pr
|
||||
- When working on a Backlog.md task, **make sure to read the implementation plan included in the task, and follow it**. If the plan is outdated compared to the conditions of the codebase, notify the user.
|
||||
- **GitHub issues are legacy and read-only for the agent.** Never create, comment on, close, or reopen GitHub issues — only the user does that. When asked about "the issue tracker" or "open issues", reach for Backlog.md, not `gh issue`.
|
||||
|
||||
### Pre-commit Hooks
|
||||
|
||||
The pre-commit hook (`scripts/dev/precommit`) inspects staged files and runs only the checks relevant to the change, rather than always running the full verification suite. CI always runs the full suite unconditionally.
|
||||
|
||||
| Category | File pattern | Checks (conditional) |
|
||||
| ----------- | -------------------------------------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
|
||||
| **elixir** | `lib/`, `test/`, `config/`, `mix.exs`, `mix.lock`, `priv/repo/migrations/`, `priv/gettext/`, `.credo.exs`, `.formatter.exs`, `.sobelow-conf` | `mix format --check-formatted`<br>`mix credo --strict`<br>`mix sobelow --compact --exit`<br>`mix gettext.extract --check-up-to-date`<br>`mix test`<br>`mix deps.unlock --unused` † |
|
||||
| **shell** | `scripts/`, `.shellcheckrc` | `shellcheck` on all `scripts/` files (excluding `.hurl`) |
|
||||
| **assets** | `assets/`, `.pi/extensions/.*\.(ts\|js\|json)$` | `prettier` on CSS, JS, and TS/JSON in `.pi/extensions/` (excluding `node_modules`) |
|
||||
| **docs** | `docs/`, `README.md`, `AGENTS.md` | `prettier` on Markdown and Livebook files |
|
||||
| **backlog** | `backlog/` | `prettier` on all backlog markdown files |
|
||||
| **presto** | `presto/` | `(cd presto && mise run test)` — runs pytest |
|
||||
| **docker** | `Dockerfile`, `.dockerignore`, `compose.yaml` | `mise run dev:validate-docker-image` (only if `Dockerfile` is staged) |
|
||||
|
||||
> **† `mix deps.unlock --unused` sub-gate**: Runs only when `mix.exs` or `mix.lock` is in the staged files — not on every Elixir change. An unused dependency can only be introduced by changing the dependency specification, not by changing application code.
|
||||
|
||||
If no staged files match any category (e.g., `git commit --allow-empty`), the script exits early without running any checks.
|
||||
|
||||
## Architecture
|
||||
|
||||
- **Context modules own all queries.** LiveViews never query the database directly -- they call context functions.
|
||||
|
||||
+55
-8
@@ -1,6 +1,6 @@
|
||||
#!/usr/bin/env bash
|
||||
|
||||
#MISE description="Run checks before a commit"
|
||||
#MISE description="Run checks before a commit (conditional on staged file types)"
|
||||
|
||||
set -e
|
||||
|
||||
@@ -8,9 +8,26 @@ source "$(git rev-parse --show-toplevel)/scripts/_helpers.sh"
|
||||
|
||||
ensure_working_directory!
|
||||
|
||||
# Convert space-separated STAGED list to newline-delimited for grep.
|
||||
# $STAGED is exported by .git/hooks/pre-commit via git diff-index.
|
||||
staged_lines() {
|
||||
echo "$STAGED" | tr ' ' '\n'
|
||||
}
|
||||
|
||||
# Early exit if no staged files (e.g., --allow-empty)
|
||||
if [ -z "$STAGED" ]; then
|
||||
debug_msg "No staged files found, skipping pre-commit checks."
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# --- Shell scripts ---
|
||||
if staged_lines | grep -qE '^scripts/|^\.shellcheckrc'; then
|
||||
debug_msg "Running shellcheck..."
|
||||
fd . 'scripts/' --exclude '*.hurl' -t file --exec shellcheck --color
|
||||
fi
|
||||
|
||||
# --- Elixir ---
|
||||
if staged_lines | grep -qE '^(lib/|test/|config/|mix\.exs|mix\.lock|priv/repo/migrations/|priv/gettext/|\.credo\.exs|\.formatter\.exs|\.sobelow-conf)'; then
|
||||
debug_msg "Running credo..."
|
||||
mix credo --strict
|
||||
|
||||
@@ -22,13 +39,43 @@ mix gettext.extract --check-up-to-date
|
||||
|
||||
debug_msg "Checking formatting..."
|
||||
mix format --check-formatted
|
||||
prettier --check '.pi/extensions/**/*.{ts,js,json}'
|
||||
prettier --check 'assets/css/**/*.css' 'assets/js/**/*.js'
|
||||
prettier --check 'docs/**/*.md' 'docs/**/*.livemd' 'README.md'
|
||||
prettier --check 'backlog/archive/**/*.md' 'backlog/completed/**/*.md' 'backlog/tasks/**/*.md' 'backlog/docs/**/*.md'
|
||||
|
||||
debug_msg "Checking unused deps..."
|
||||
mix deps.unlock --unused
|
||||
|
||||
debug_msg "Running tests..."
|
||||
mix test
|
||||
fi
|
||||
|
||||
# deps.unlock sub-gate: only when mix.exs or mix.lock changed
|
||||
if staged_lines | grep -qE '^mix\.exs$|^mix\.lock$'; then
|
||||
debug_msg "Checking unused deps..."
|
||||
mix deps.unlock --unused
|
||||
fi
|
||||
|
||||
# --- Assets (JS/TS/CSS) ---
|
||||
if staged_lines | grep -qE '^assets/|^\.pi/extensions/.*\.(ts|js|json)$'; then
|
||||
debug_msg "Checking assets formatting..."
|
||||
prettier --check 'assets/css/**/*.css' 'assets/js/**/*.js' '.pi/extensions/**/*.{ts,js,json}' '!.pi/extensions/**/node_modules/**'
|
||||
fi
|
||||
|
||||
# --- Documentation ---
|
||||
if staged_lines | grep -qE '^docs/|^README\.md$|^AGENTS\.md$'; then
|
||||
debug_msg "Checking docs formatting..."
|
||||
prettier --check 'docs/**/*.md' 'docs/**/*.livemd' 'README.md' 'AGENTS.md'
|
||||
fi
|
||||
|
||||
# --- Backlog ---
|
||||
if staged_lines | grep -qE '^backlog/'; then
|
||||
debug_msg "Checking backlog formatting..."
|
||||
prettier --check 'backlog/archive/**/*.md' 'backlog/completed/**/*.md' 'backlog/tasks/**/*.md' 'backlog/docs/**/*.md'
|
||||
fi
|
||||
|
||||
# --- Presto ---
|
||||
if staged_lines | grep -qE '^presto/'; then
|
||||
debug_msg "Checking presto..."
|
||||
(cd presto && mise run test)
|
||||
fi
|
||||
|
||||
# --- Docker ---
|
||||
if staged_lines | grep -qE '^Dockerfile$|^\.dockerignore$|^compose\.yaml$'; then
|
||||
debug_msg "Validating Docker image..."
|
||||
mise run dev:validate-docker-image
|
||||
fi
|
||||
|
||||
Reference in New Issue
Block a user