Skip to content

Commit ed0b463

Browse files
committed
fix(ci): fail closed and exclude root-level vendor dirs
Address review feedback on the boilerplate checker: - Exclusion globs like '*/node_modules/*' never matched a path starting at the repository root; list each vendored directory in both forms. - A missing merge base (shallow clone) printed a warning and exited 0, which silently disabled the gate. Fail with an actionable message. - Collect the file list into a variable first; process substitution ran collect_files in a subshell and swallowed its non-zero exit. Signed-off-by: FenjuFu <92919259+FenjuFu@users.noreply.github.com>
1 parent 188625f commit ed0b463

1 file changed

Lines changed: 24 additions & 8 deletions

File tree

makefiles/check-boilerplate.sh

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -75,11 +75,17 @@ while [[ $# -gt 0 ]]; do
7575
esac
7676
done
7777

78-
# Paths that are vendored, generated or otherwise not ours to license
78+
# Paths that are vendored, generated or otherwise not ours to license.
79+
# Each directory is listed twice so it matches at the repository root as well
80+
# as nested, because '*/name/*' cannot match a path that starts with 'name/'.
7981
is_excluded() {
8082
case "$1" in
81-
*/node_modules/*|*/dist/*|*/build/*|*/target/*|*/.venv/*|*/venv/*) return 0 ;;
82-
*/__pycache__/*|*/migrations/*|helm/*|*/generated/*|*.min.js) return 0 ;;
83+
node_modules/*|*/node_modules/*) return 0 ;;
84+
dist/*|*/dist/*|build/*|*/build/*|target/*|*/target/*) return 0 ;;
85+
.venv/*|*/.venv/*|venv/*|*/venv/*) return 0 ;;
86+
__pycache__/*|*/__pycache__/*) return 0 ;;
87+
migrations/*|*/migrations/*|generated/*|*/generated/*) return 0 ;;
88+
helm/*|*.min.js) return 0 ;;
8389
*) return 1 ;;
8490
esac
8591
}
@@ -104,11 +110,15 @@ collect_files() {
104110
git -C "$REPO_ROOT" ls-files
105111
else
106112
local merge_base
107-
if merge_base="$(git -C "$REPO_ROOT" merge-base "$BASE_REF" HEAD 2>/dev/null)"; then
108-
git -C "$REPO_ROOT" diff --diff-filter=A --name-only "$merge_base" HEAD
109-
else
110-
echo -e "${YELLOW}⚠️ Cannot resolve $BASE_REF, nothing to check${RESET}" >&2
113+
if ! merge_base="$(git -C "$REPO_ROOT" merge-base "$BASE_REF" HEAD 2>/dev/null)"; then
114+
# Failing closed matters: a shallow clone would otherwise check
115+
# nothing and report success, silently disabling the CI gate
116+
echo -e "${RED}❌ Cannot resolve a merge base with $BASE_REF.${RESET}" >&2
117+
echo -e "${YELLOW} Fetch the base branch first (fetch-depth: 0 in GitHub Actions),${RESET}" >&2
118+
echo -e "${YELLOW} or pass --all to scan every tracked file instead.${RESET}" >&2
119+
return 1
111120
fi
121+
git -C "$REPO_ROOT" diff --diff-filter=A --name-only "$merge_base" HEAD
112122
fi
113123
}
114124

@@ -195,6 +205,12 @@ missing=0
195205
checked=0
196206
fixed=0
197207

208+
# Collect first rather than piping through process substitution: a subshell
209+
# would swallow a non-zero exit from collect_files and report a false pass
210+
if ! files_list="$(collect_files)"; then
211+
exit 1
212+
fi
213+
198214
while IFS= read -r file; do
199215
[[ -z "$file" || ! -f "$REPO_ROOT/$file" ]] && continue
200216
is_excluded "$file" && continue
@@ -215,7 +231,7 @@ while IFS= read -r file; do
215231
echo -e "${RED} ✗ missing header: $file${RESET}"
216232
missing=$((missing + 1))
217233
fi
218-
done < <(collect_files)
234+
done <<< "$files_list"
219235

220236
echo ""
221237
echo -e "${BLUE} Files inspected: $checked${RESET}"

0 commit comments

Comments
 (0)