Skip to content

Commit f31e298

Browse files
justin808claude
andcommitted
Improve documentation and reduce duplication in docs-only guard
Address code review feedback: - Clarify API rate limit constraints in 7-day window comment - Document run_number tiebreaker behavior for out-of-order reruns - Explain why 'cancelled' workflows block docs-only skips - Eliminate duplication in ci-changes-detector using CI_FLAGS array 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 882174d commit f31e298

File tree

2 files changed

+38
-26
lines changed

2 files changed

+38
-26
lines changed

.github/actions/ensure-master-docs-safety/action.yml

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@ runs:
2222
// Query workflow runs from the last 7 days to avoid excessive API calls.
2323
// Why 7 days? This balances API efficiency with practical needs:
2424
// - Most master commits trigger CI within hours, not days
25-
// - Commits older than 7 days are likely stale; better to run full CI anyway
25+
// - Commits older than 7 days are likely stale; better to allow the docs-only skip anyway
2626
// - Reduces pagination load on high-velocity repos
27+
// - GitHub API rate limits (1000 req/hr for Actions) make unbounded searches risky
2728
// For commits outside this window, we skip the check and allow the docs-only skip.
2829
const createdAfter = new Date(Date.now() - 1000 * 60 * 60 * 24 * 7).toISOString();
2930
@@ -67,6 +68,8 @@ runs:
6768
// Deduplicate workflow runs by keeping only the latest run for each workflow_id.
6869
// This handles cases where workflows are re-run manually.
6970
// Use run_number as tiebreaker since created_at might be identical for rapid reruns.
71+
// Note: If workflows are manually re-run out of order, we use the highest run_number
72+
// which represents the most recent attempt, regardless of trigger order.
7073
const latestByWorkflow = new Map();
7174
for (const run of workflowRuns) {
7275
const existing = latestByWorkflow.get(run.workflow_id);
@@ -101,7 +104,9 @@ runs:
101104
// We treat these conclusions as failures:
102105
// - 'failure': Obvious failure case
103106
// - 'timed_out': Infrastructure or performance issue that should be investigated
104-
// - 'cancelled': Conservative - might indicate timeout or manual intervention needed
107+
// - 'cancelled': Might indicate timeout, CI infrastructure issues, or manual intervention needed
108+
// Being conservative here prevents a green checkmark when the previous commit
109+
// might have real issues that weren't fully validated
105110
// - 'action_required': Requires manual intervention
106111
// We treat 'skipped' and 'neutral' as non-blocking since they indicate
107112
// intentional skips or informational-only workflows.

script/ci-changes-detector

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -147,34 +147,41 @@ echo "Base: $BASE_REF | Current: $CURRENT_REF"
147147
echo ""
148148

149149
if [ "$DOCS_ONLY" = true ]; then
150+
# Define all CI flags in one place to avoid duplication
151+
# Format: "flag_name:value" pairs
152+
CI_FLAGS=(
153+
"docs_only:true"
154+
"run_lint:false"
155+
"run_ruby_tests:false"
156+
"run_js_tests:false"
157+
"run_dummy_tests:false"
158+
"run_generators:false"
159+
"run_pro_lint:false"
160+
"run_pro_tests:false"
161+
"run_pro_dummy_tests:false"
162+
)
163+
164+
# Output to GITHUB_OUTPUT if in GitHub Actions
150165
if [ -n "${GITHUB_OUTPUT:-}" ]; then
151-
{
152-
echo "docs_only=true"
153-
echo "run_lint=false"
154-
echo "run_ruby_tests=false"
155-
echo "run_js_tests=false"
156-
echo "run_dummy_tests=false"
157-
echo "run_generators=false"
158-
echo "run_pro_lint=false"
159-
echo "run_pro_tests=false"
160-
echo "run_pro_dummy_tests=false"
161-
} >> "$GITHUB_OUTPUT"
166+
for flag in "${CI_FLAGS[@]}"; do
167+
echo "${flag//:/=}" >> "$GITHUB_OUTPUT"
168+
done
162169
fi
163170

171+
# Output as JSON if requested
164172
if [ "${CI_JSON_OUTPUT:-}" = "1" ]; then
165-
cat << EOF
166-
{
167-
"docs_only": true,
168-
"run_lint": false,
169-
"run_ruby_tests": false,
170-
"run_js_tests": false,
171-
"run_dummy_tests": false,
172-
"run_generators": false,
173-
"run_pro_lint": false,
174-
"run_pro_tests": false,
175-
"run_pro_dummy_tests": false
176-
}
177-
EOF
173+
echo "{"
174+
for i in "${!CI_FLAGS[@]}"; do
175+
flag_name="${CI_FLAGS[$i]%%:*}"
176+
flag_value="${CI_FLAGS[$i]#*:}"
177+
# Add comma for all but the last item
178+
if [ "$i" -eq $((${#CI_FLAGS[@]} - 1)) ]; then
179+
echo " \"$flag_name\": $flag_value"
180+
else
181+
echo " \"$flag_name\": $flag_value,"
182+
fi
183+
done
184+
echo "}"
178185
fi
179186

180187
echo -e "${GREEN}✓ Documentation-only changes${NC}"

0 commit comments

Comments
 (0)