docs: allow approved third-party packages #59
@@ -2,8 +2,26 @@
|
||||
|
||||
## Language & Dependencies
|
||||
|
||||
- Go standard library only — no external dependencies.
|
||||
- Target the latest stable Go release.
|
||||
- **STRICT ALLOWLIST:** Only packages listed below may be imported. No exceptions.
|
||||
|
||||
### Approved Third-Party Packages
|
||||
|
||||
| Package | Use Case | Scope |
|
||||
|---------|----------|-------|
|
||||
|
|
||||
| `gopkg.in/yaml.v3` | YAML parsing (persona files, config) | production |
|
||||
sonnet-review-bot
commented
[NIT] **[NIT]** `go-cmp` is a test-only dependency, but the table doesn't distinguish test dependencies from production dependencies. A 'Scope' column (e.g. 'test only' vs 'production') would clarify that `go-cmp` should never appear in non-test code, preventing accidental production use.
|
||||
| `github.com/google/go-cmp` | Test comparisons (`cmp.Diff`) | test only |
|
||||
gpt-review-bot
commented
[MINOR] The dependency request process omits updating the enforcement mechanism. If the allowlist remains hard-coded in scripts/check-deps.sh, instructions should explicitly require updating the script alongside the table, or the script should be changed to derive its allowlist from a single canonical source. **[MINOR]** The dependency request process omits updating the enforcement mechanism. If the allowlist remains hard-coded in scripts/check-deps.sh, instructions should explicitly require updating the script alongside the table, or the script should be changed to derive its allowlist from a single canonical source.
|
||||
|
||||
|
sonnet-review-bot
commented
[NIT] The table lists **[NIT]** The table lists `github.com/google/go-cmp` with scope 'test only', but the enforcement script (`check-deps.sh`) does not differentiate between production and test-only scope — it checks all direct go.mod dependencies uniformly. If `go-cmp` ends up in go.mod as a direct dependency (which it will when used in `_test.go` files), it passes the allowlist check regardless of where it's imported. The 'test only' scope annotation is purely documentation with no mechanical enforcement. This is acceptable but worth noting so future maintainers don't assume it's enforced.
sonnet-review-bot
commented
[NIT] The note 'Transitive dependencies of approved packages are automatically allowed' is a policy statement, but the enforcement script only checks direct module dependencies (via **[NIT]** The note 'Transitive dependencies of approved packages are automatically allowed' is a policy statement, but the enforcement script only checks direct module dependencies (via `go list -m ... all` with `.Indirect` filtered out). This is correct and intentional, but it's worth confirming the wording matches: transitive deps won't appear as violations, which aligns with the statement.
|
||||
**Any import not in this table or the Go standard library is forbidden.**
|
||||
|
sonnet-review-bot
commented
[NIT] The process for adding new dependencies doesn't mention who has approval authority (e.g. maintainers, a specific review process). Adding a brief note like 'requires approval from a maintainer' would make the governance expectation explicit. **[NIT]** The process for adding new dependencies doesn't mention who has approval authority (e.g. maintainers, a specific review process). Adding a brief note like 'requires approval from a maintainer' would make the governance expectation explicit.
|
||||
|
||||
Transitive dependencies of approved packages are automatically allowed.
|
||||
|
||||
To request a new dependency:
|
||||
1. Open a PR that ONLY updates this table
|
||||
|
sonnet-review-bot
commented
[NIT] The sentence 'Transitive dependencies of approved packages are automatically allowed' is a policy statement that the check-deps.sh script does NOT currently verify (it only checks direct deps via **[NIT]** The sentence 'Transitive dependencies of approved packages are automatically allowed' is a policy statement that the check-deps.sh script does NOT currently verify (it only checks direct deps via `go list -m`). This is correct behavior—you generally don't want to enumerate all transitive deps—but the wording could be clearer: something like 'Transitive dependencies pulled in by approved packages do not need to be listed here' to make it clear this is intentional, not an oversight.
|
||||
2. Requires explicit approval from Aaron
|
||||
3. After merge, a separate PR may use the package
|
||||
|
||||
*Enforcement: `scripts/check-deps.sh` parses this table — update only here.*
|
||||
|
||||
## Error Handling
|
||||
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
.PHONY: build test test-integration lint clean coverage
|
||||
.PHONY: build test test-integration lint clean coverage check-deps precommit
|
||||
|
gpt-review-bot
commented
[MINOR] The new 'precommit' target isn't listed in .PHONY. It should be added to avoid make treating a file named 'precommit' as up-to-date and skipping the target. **[MINOR]** The new 'precommit' target isn't listed in .PHONY. It should be added to avoid make treating a file named 'precommit' as up-to-date and skipping the target.
|
||||
|
||||
build:
|
||||
go build -o review-bot ./cmd/review-bot/
|
||||
@@ -12,9 +12,15 @@ test-integration:
|
||||
lint:
|
||||
go vet ./...
|
||||
|
||||
check-deps:
|
||||
|
gpt-review-bot
commented
[NIT] The check-deps script requires Bash 4+. On macOS, default /bin/bash is 3.x; while the script emits guidance, you might consider documenting this requirement alongside the new precommit target for developer setup clarity. **[NIT]** The check-deps script requires Bash 4+. On macOS, default /bin/bash is 3.x; while the script emits guidance, you might consider documenting this requirement alongside the new precommit target for developer setup clarity.
|
||||
@./scripts/check-deps.sh
|
||||
|
||||
clean:
|
||||
rm -f review-bot
|
||||
|
||||
coverage:
|
||||
go test -coverprofile=coverage.out ./...
|
||||
go tool cover -func=coverage.out
|
||||
|
||||
# Precommit runs all checks required before pushing
|
||||
precommit: check-deps lint test
|
||||
[MINOR] The new **[MINOR]** The new `precommit` target runs the dependency check, but unless CI invokes this target, policy enforcement may not occur in CI. Ensure CI runs `make precommit` (or otherwise invokes `check-deps`) to prevent unapproved dependencies from being merged.
|
||||
|
||||
@@ -0,0 +1,127 @@
|
||||
#!/usr/bin/env bash
|
||||
# check-deps.sh - Enforces the strict dependency allowlist from CONVENTIONS.md
|
||||
# Exit 1 if any unapproved import is found.
|
||||
#
|
||||
# Requires: Bash 4+ (for associative arrays), Go toolchain
|
||||
#
|
||||
# The allowlist is parsed from CONVENTIONS.md to maintain a single source of truth.
|
||||
# Enforces Scope column: "test only" packages cannot appear in non-test code.
|
||||
|
[MINOR] The allowlist is duplicated between CONVENTIONS.md and this script ( **[MINOR]** The allowlist is duplicated between CONVENTIONS.md and this script (`ALLOWED` array), creating a risk of drift. A mismatch could either block approved deps or unintentionally allow unapproved ones, weakening policy enforcement. Consider deriving the list from a single source (e.g., parse CONVENTIONS.md or define the list in a single machine-readable file).
gpt-review-bot
commented
[MAJOR] The allowlist is hard-coded in the script (ALLOWED array) while CONVENTIONS.md is presented as the source of truth. The documented process says to open a PR that ONLY updates the table, but the enforcement will still fail later unless this script is updated too. This mismatch will cause policy drift and block subsequent PRs using newly approved dependencies. **[MAJOR]** The allowlist is hard-coded in the script (ALLOWED array) while CONVENTIONS.md is presented as the source of truth. The documented process says to open a PR that ONLY updates the table, but the enforcement will still fail later unless this script is updated too. This mismatch will cause policy drift and block subsequent PRs using newly approved dependencies.
|
||||
|
||||
set -euo pipefail
|
||||
|
||||
# Check bash version
|
||||
if ((BASH_VERSINFO[0] < 4)); then
|
||||
echo "❌ Bash 4+ required (found ${BASH_VERSION})"
|
||||
|
sonnet-review-bot
commented
[MINOR] The ALLOWED_PATTERN variable is built but never used. The actual checking is done in the loop with direct string comparison against the ALLOWED array. The dead code is harmless but confusing — either use the pattern (e.g., with grep) or remove it. **[MINOR]** The ALLOWED_PATTERN variable is built but never used. The actual checking is done in the loop with direct string comparison against the ALLOWED array. The dead code is harmless but confusing — either use the pattern (e.g., with grep) or remove it.
|
||||
echo " On macOS: brew install bash"
|
||||
|
gpt-review-bot
commented
[NIT] ALLOWED_PATTERN is built but never used. Remove this dead code to reduce confusion and keep the script minimal. **[NIT]** ALLOWED_PATTERN is built but never used. Remove this dead code to reduce confusion and keep the script minimal.
|
||||
exit 1
|
||||
fi
|
||||
|
||||
CONVENTIONS_FILE="${1:-CONVENTIONS.md}"
|
||||
|
||||
|
gpt-review-bot
commented
[MAJOR] The script does not enforce the 'Scope' column from CONVENTIONS.md (e.g., allowing github.com/google/go-cmp only in tests). As written, any listed package is allowed in production code, contradicting the documented policy. **[MAJOR]** The script does not enforce the 'Scope' column from CONVENTIONS.md (e.g., allowing github.com/google/go-cmp only in tests). As written, any listed package is allowed in production code, contradicting the documented policy.
gpt-review-bot
commented
[MINOR] The script relies on bash associative arrays ( **[MINOR]** The script relies on bash associative arrays (`declare -A`), which require Bash 4+. On macOS (default Bash 3.2), this will fail locally for contributors running `make precommit`. Consider documenting the requirement, invoking a known shell (e.g., via `env bash` with ensured version), or rewriting to avoid associative arrays.
|
||||
if [ ! -f "$CONVENTIONS_FILE" ]; then
|
||||
echo "❌ CONVENTIONS.md not found"
|
||||
|
sonnet-review-bot
commented
[NIT] The comment says 'POSIX-compatible' but **[NIT]** The comment says 'POSIX-compatible' but `declare -A` (associative arrays) is a bash 4+ feature and is not POSIX. The comment should say 'bash 4+' to accurately describe the requirement. macOS ships bash 3.2 by default (GPLv2 licensing constraint), so this script would fail on stock macOS without Homebrew bash. Worth documenting the bash version requirement in a shebang comment or README.
|
||||
exit 1
|
||||
|
gpt-review-bot
commented
[MINOR] Uses 'grep -P' (Perl regex), which is not available on macOS/BSD grep by default. This reduces cross-platform developer usability for the precommit hook. **[MINOR]** Uses 'grep -P' (Perl regex), which is not available on macOS/BSD grep by default. This reduces cross-platform developer usability for the precommit hook.
|
||||
fi
|
||||
|
[MINOR] The script fails open if **[MINOR]** The script fails open if `go list -m all` errors (`|| true`), then treats an empty IMPORTS as "no external dependencies" and exits 0. This can bypass enforcement if the Go toolchain is unavailable or the command fails. Prefer failing closed on errors or explicitly erroring when `go list` fails.
|
||||
|
||||
# Parse approved packages from CONVENTIONS.md table using awk (POSIX-compatible)
|
||||
|
gpt-review-bot
commented
[MINOR] Table parsing relies on a brittle grep pattern ('^| `[a-zA-Z]') and a single-code-cell extraction. Future formatting changes (leading spaces, additional code spans) or package names starting with non-letters may cause false negatives/positives. **[MINOR]** Table parsing relies on a brittle grep pattern ('^\| `[a-zA-Z]') and a single-code-cell extraction. Future formatting changes (leading spaces, additional code spans) or package names starting with non-letters may cause false negatives/positives.
|
||||
# Format: | `package` | use case | scope |
|
||||
|
sonnet-review-bot
commented
[MINOR] **[MINOR]** `go list -m all` includes indirect/transitive dependencies. If yaml.v3 or go-cmp pull in their own transitive deps, those would appear here and trigger false-positive violations. Consider using `go mod edit -json | jq '.Require[] | select(.Indirect == false) | .Path'` or filtering with `go list -m -mod=mod -f '{{if not .Indirect}}{{.Path}}{{end}}' all` to check only direct dependencies.
|
||||
declare -A ALLOWED_PROD=()
|
||||
declare -A ALLOWED_TEST=()
|
||||
|
||||
while IFS= read -r line; do
|
||||
# Use awk to extract package and scope from table row
|
||||
|
gpt-review-bot
commented
[NIT] Parsing the markdown table with **[NIT]** Parsing the markdown table with `grep`/`awk` is somewhat brittle (e.g., relies on backticks around package names and exact column positions). This is acceptable given the documented process, but a brief note in CONVENTIONS.md to preserve formatting would help avoid accidental breakage.
|
||||
pkg=$(echo "$line" | awk -F'|' '{gsub(/^[[:space:]]*`|`[[:space:]]*$/, "", $2); print $2}')
|
||||
scope=$(echo "$line" | awk -F'|' '{gsub(/^[[:space:]]+|[[:space:]]+$/, "", $4); print tolower($4)}')
|
||||
|
sonnet-review-bot
commented
[MINOR] The **[MINOR]** The `-oP` flag for PCRE-style regex (`\K`) is a GNU grep extension and is not available on macOS (BSD grep). This will silently fail to parse the allowlist on macOS developer machines (the `|| true` swallows the error), leaving ALLOWED empty and triggering the warning but not enforcing anything. Consider using `sed` instead: `echo "$line" | sed -n "s/^| \`\([^\`]*\)\`.*/\1/p"`, or document that GNU grep is required.
|
||||
|
||||
|
gpt-review-bot
commented
[MINOR] The filter **[MINOR]** The filter `[[ "$pkg" =~ ^[a-zA-Z] ]]` rejects valid import paths that begin with a digit (e.g., 9fans.net/go). Consider relaxing to `^[[:alnum:]]` or removing the check, since the header row is already excluded by the grep.
|
||||
if [ -n "$pkg" ] && [ "$pkg" != "Package" ] && [[ "$pkg" =~ ^[a-zA-Z] ]]; then
|
||||
if [[ "$scope" == *"test"* ]]; then
|
||||
ALLOWED_TEST["$pkg"]=1
|
||||
else
|
||||
ALLOWED_PROD["$pkg"]=1
|
||||
fi
|
||||
fi
|
||||
done < <(grep '| `' "$CONVENTIONS_FILE" 2>/dev/null || true)
|
||||
|
||||
|
sonnet-review-bot
commented
[MINOR] **[MINOR]** `go list -m -f '{{if not .Indirect}}{{.Path}}{{end}}' all` lists module-level direct dependencies, not package-level imports. This means a module in go.mod that isn't actually imported in any Go source file will still be flagged. More precisely, it will miss the case where someone adds a `go.mod` entry for an unapproved module but the script will catch modules not in go.mod at all (since they wouldn't be listed). This is probably fine for the intended purpose, but the comment 'DIRECT dependencies only' may mislead — it means direct in the go.mod sense, not 'actually used in source'. Consider a brief clarifying comment.
|
||||
ALL_ALLOWED=("${!ALLOWED_PROD[@]}" "${!ALLOWED_TEST[@]}")
|
||||
|
||||
if [ ${#ALL_ALLOWED[@]} -eq 0 ]; then
|
||||
echo "⚠️ No approved packages found in $CONVENTIONS_FILE"
|
||||
echo " (This is fine if you want stdlib-only)"
|
||||
fi
|
||||
|
||||
|
sonnet-review-bot
commented
[NIT] **[NIT]** `echo -e` is not POSIX-compliant and behavior varies across shells (e.g., on some systems /bin/bash may not support -e with printf semantics). The `\n` in the violations string also won't render on all platforms. Consider using `printf` instead: `printf 'The following imports are not in the allowlist:%b\n' "$VIOLATIONS"`.
|
||||
# Helper: check if import matches any package in an associative array (literal prefix, no glob)
|
||||
matches_allowlist() {
|
||||
local import="$1"
|
||||
|
[MINOR] The prefix match uses , which treats the right side as a glob pattern. While allowed entries are controlled via review, a metacharacter (e.g., *, ?, [) in an approved package could unintentionally broaden matches. Consider a literal prefix check to avoid glob semantics. **[MINOR]** The prefix match uses [[ "$import" == "$allowed/"* ]], which treats the right side as a glob pattern. While allowed entries are controlled via review, a metacharacter (e.g., *, ?, [) in an approved package could unintentionally broaden matches. Consider a literal prefix check to avoid glob semantics.
|
||||
shift
|
||||
local -n allowlist=$1
|
||||
|
||||
|
sonnet-review-bot
commented
[NIT] The comment says 'POSIX-compatible' for the awk parsing but the outer loop uses a Bash process substitution **[NIT]** The comment says 'POSIX-compatible' for the awk parsing but the outer loop uses a Bash process substitution `< <(...)` which is Bash-specific. The comment is mildly misleading — it means awk itself is POSIX, not the overall approach.
|
||||
for allowed in "${!allowlist[@]}"; do
|
||||
# Exact match
|
||||
if [ "$import" = "$allowed" ]; then
|
||||
return 0
|
||||
fi
|
||||
# Literal prefix match for subpackages: must match "pkg/" exactly
|
||||
if [ "${import#"$allowed/"}" != "$import" ]; then
|
||||
return 0
|
||||
|
gpt-review-bot
commented
[MAJOR] The script checks only direct non-test modules via **[MAJOR]** The script checks only direct non-test modules via `go list -m ... all` and never verifies test-only dependencies against the allowlist. As a result, importing any new third-party package in tests (e.g., in *_test.go) will not be flagged unless also used in production code. This violates the documented 'STRICT ALLOWLIST' policy for tests.
|
||||
fi
|
||||
done
|
||||
return 1
|
||||
|
sonnet-review-bot
commented
[MINOR] The **[MINOR]** The `go list -m` output includes the current module itself on the first line, and the comment says 'Filter out the module itself (first line)'. However, `tail -n +2` strips the first line unconditionally. If `go list -m ... all` ever reorders output or includes a blank first line (e.g., when run in a workspace), this assumption breaks silently. A more robust approach would be to filter by comparing against `$(go list -m)` (the current module path) rather than relying on line position.
|
||||
}
|
||||
|
||||
# Get direct module dependencies from go.mod
|
||||
|
sonnet-review-bot
commented
[MINOR] If **[MINOR]** If `go list` returns non-module output (e.g. build errors) the error is captured in `DIRECT_IMPORTS` and the early-exit error message will contain the raw go toolchain output. This works adequately but the error message could be confusing. Minor quality-of-life issue.
|
||||
DIRECT_IMPORTS=$(go list -m -f '{{if and (not .Indirect) (not .Main)}}{{.Path}}{{end}}' all 2>&1) || {
|
||||
echo "❌ Failed to list dependencies: $DIRECT_IMPORTS"
|
||||
|
sonnet-review-bot
commented
[MINOR] The script checks **[MINOR]** The script checks `go.mod` direct dependencies for allowlist compliance, but the scope enforcement (lines 97-111) checks the full production import graph via `go list -deps`. These two checks are at different granularities and could diverge. For example, a direct dependency might be test-only in practice but `go list -deps ./...` (which doesn't filter test files) would still traverse it. Using `go list -deps -test=false ./...` more explicitly conveys intent, though the current flag-less form already excludes test builds.
|
||||
exit 1
|
||||
}
|
||||
DIRECT_IMPORTS=$(echo "$DIRECT_IMPORTS" | grep -v '^$' || true)
|
||||
|
||||
if [ -z "$DIRECT_IMPORTS" ]; then
|
||||
echo "✅ No external dependencies"
|
||||
exit 0
|
||||
fi
|
||||
|
||||
# Check ALL direct dependencies are in some allowlist
|
||||
VIOLATIONS=""
|
||||
|
sonnet-review-bot
commented
[NIT] The final line counts direct deps via **[NIT]** The final line counts direct deps via `wc -l` on `$IMPORTS`, but `$IMPORTS` could contain blank lines depending on the `go list` output format when some template expansions produce empty strings. The `grep -v '^$'` earlier helps but the count may still be off by one if there's a trailing newline. Minor cosmetic issue.
|
||||
while IFS= read -r import; do
|
||||
[ -z "$import" ] && continue
|
||||
|
||||
if ! matches_allowlist "$import" ALLOWED_PROD && ! matches_allowlist "$import" ALLOWED_TEST; then
|
||||
VIOLATIONS="${VIOLATIONS} - ${import} (not in allowlist)"$'\n'
|
||||
fi
|
||||
done <<< "$DIRECT_IMPORTS"
|
||||
|
||||
if [ -n "$VIOLATIONS" ]; then
|
||||
echo "❌ UNAPPROVED DEPENDENCIES DETECTED"
|
||||
echo ""
|
||||
echo "The following imports are not in the allowlist:"
|
||||
printf "%s" "$VIOLATIONS"
|
||||
echo ""
|
||||
echo "To add a dependency, update CONVENTIONS.md (requires Aaron's approval)"
|
||||
|
sonnet-review-bot
commented
[MINOR] The test-scope enforcement uses **[MINOR]** The test-scope enforcement uses `go list -deps -f '...' ./... | grep -v '_test'` to identify production imports, but this approach is fragile: it filters by path containing '_test', which catches test packages but the `-deps` flag on non-test packages already excludes test files. More importantly, this will fail to detect a test-only package imported via a non-test file in a sub-package whose path happens not to contain '_test'. Using `go list -f '{{if not .Standard}}{{.ImportPath}}{{end}}' $(go list ./...)` (without `-deps`) and then checking against the allowlist directly would be more precise. That said, the current approach is a reasonable heuristic.
gpt-review-bot
commented
[NIT] The **[NIT]** The `grep -v '_test'` in the production imports step is unnecessary because `go list -deps` without `-test` already excludes test-only dependencies. While harmless, it can be removed for clarity.
|
||||
exit 1
|
||||
fi
|
||||
|
[MINOR] Production imports discovery allows go list to fail open (2>/dev/null | ... || true). If go list fails, PROD_IMPORTS becomes empty and the script won’t detect misuse of test-only dependencies, weakening enforcement. Failing closed here would improve robustness. **[MINOR]** Production imports discovery allows go list to fail open (2>/dev/null | ... || true). If go list fails, PROD_IMPORTS becomes empty and the script won’t detect misuse of test-only dependencies, weakening enforcement. Failing closed here would improve robustness.
|
||||
|
||||
# Enforce Scope: test-only packages must not appear in non-test code
|
||||
|
gpt-review-bot
commented
[MINOR] The scope enforcement uses **[MINOR]** The scope enforcement uses `grep -q "^${test_pkg}"` which can produce false positives for modules with a similar prefix (e.g., github.com/google/go-cmppro would match github.com/google/go-cmp). Use a delimiter-aware pattern like `^${test_pkg}(/|$)` to avoid accidental matches.
|
||||
# Get imports used by non-test code only (go list -deps without -test excludes test deps)
|
||||
|
sonnet-review-bot
commented
[MINOR] The regex **[MINOR]** The regex `"^${test_pkg}(/|\$|$)"` has a redundant `$` — `\$` escapes the literal dollar sign for the shell but in the regex context inside `grep -qE`, the trailing `|$)` is a proper end-of-line anchor followed by the closing group, making `\$` (literal `$` character) unreachable in practice. This is harmless but the intent (match exact package or subpackage) is already handled by the `^pkg($|/)` pattern. Consider simplifying to `"^${test_pkg}(/|$)"`.
|
||||
PROD_IMPORTS=$(go list -deps -f '{{if not .Standard}}{{.ImportPath}}{{end}}' ./... 2>/dev/null || true)
|
||||
|
[MINOR] The grep check for test-only packages in production uses an unescaped regex pattern (grep -q "^${test_pkg}"). If a package name contains regex metacharacters (e.g., dots), this may match unintended imports. While not a code execution risk, it can cause false positives/negatives in enforcement. **[MINOR]** The grep check for test-only packages in production uses an unescaped regex pattern (grep -q "^${test_pkg}"). If a package name contains regex metacharacters (e.g., dots), this may match unintended imports. While not a code execution risk, it can cause false positives/negatives in enforcement.
|
||||
|
||||
TEST_ONLY_IN_PROD=""
|
||||
for test_pkg in "${!ALLOWED_TEST[@]}"; do
|
||||
|
sonnet-review-bot
commented
[MINOR] The test-only production check uses **[MINOR]** The test-only production check uses `grep -q "^${test_pkg}"` without anchoring the end of the pattern. A package path like `github.com/google/go-cmp` would inadvertently match a hypothetical `github.com/google/go-cmp-extended`. Should be `grep -qE "^${test_pkg}(/|$)"` to match only exact or subpackage imports.
|
||||
# Use word-boundary matching: exact match or followed by /
|
||||
|
gpt-review-bot
commented
[MINOR] The grep regex **[MINOR]** The grep regex `^${test_pkg}(/|\$|$)` redundantly includes both `\$` and `$`. It can be simplified to `^${test_pkg}(/|$)` without changing semantics.
|
||||
if echo "$PROD_IMPORTS" | grep -qE "^${test_pkg}(/|\$|$)"; then
|
||||
TEST_ONLY_IN_PROD="${TEST_ONLY_IN_PROD} - ${test_pkg} (marked 'test only' but used in production code)"$'\n'
|
||||
fi
|
||||
done
|
||||
|
||||
if [ -n "$TEST_ONLY_IN_PROD" ]; then
|
||||
echo "❌ TEST-ONLY DEPENDENCIES IN PRODUCTION CODE"
|
||||
echo ""
|
||||
printf "%s" "$TEST_ONLY_IN_PROD"
|
||||
echo ""
|
||||
echo "These packages are marked 'test only' in CONVENTIONS.md"
|
||||
echo "and must only be imported from *_test.go files."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
echo "✅ All dependencies are approved"
|
||||
echo " Direct module deps: $(echo "$DIRECT_IMPORTS" | wc -l | tr -d ' ')"
|
||||
echo " Production allowlist: ${#ALLOWED_PROD[@]}, Test-only allowlist: ${#ALLOWED_TEST[@]}"
|
||||
[MINOR] Policy text says 'Only packages listed below may be imported' but the enforcement checks modules via 'go list -m all' and will also block any transitive modules not on the list. Consider clarifying that approving a dependency may entail approving its transitive modules, or adjust the script to allow transitive deps of approved modules automatically.