feat: ignore test_and_release in PRs without code changes#6974
feat: ignore test_and_release in PRs without code changes#6974robertolopezlopez wants to merge 1 commit into
Conversation
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
robertolopezlopez
force-pushed
the
feat/CLI-1625
branch
3 times, most recently
from
4944ced to
5c91d33
Compare
robertolopezlopez
changed the title
robertolopezlopez
force-pushed
the
feat/CLI-1625
branch
from
5d2253f to
506ea98
Compare
|
/describe |
|
PR Description updated to latest commit (506ea98) |
robertolopezlopez
force-pushed
the
feat/CLI-1625
branch
from
506ea98 to
75e5ac5
Compare
This comment has been minimized.
This comment has been minimized.
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| if ./scripts/ci/has-code-changes.sh; then | ||
| echo '.circleci/continue_config.yml' > /tmp/continuation-config-path.txt | ||
| else | ||
| echo '.circleci/continue_config_light.yml' > /tmp/continuation-config-path.txt | ||
| fi |
There was a problem hiding this comment.
Error handling bug: If has-code-changes.sh exits with error code 2 (e.g., when git merge-base fails due to insufficient fetch depth), the else branch executes and selects the lightweight config instead of failing the build. This silently masks errors and could run the wrong pipeline.
Impact: On older PR branches where the merge-base is beyond the 100-commit fetch depth (line 17), the detection will error but incorrectly proceed with lightweight config, skipping critical tests.
Fix:
set +e
./scripts/ci/has-code-changes.sh
result=$?
set -e
if [ $result -eq 0 ]; then
echo '.circleci/continue_config.yml' > /tmp/continuation-config-path.txt
elif [ $result -eq 1 ]; then
echo '.circleci/continue_config_light.yml' > /tmp/continuation-config-path.txt
else
echo "Error detecting code changes (exit $result)" >&2
exit 1
fi| if ./scripts/ci/has-code-changes.sh; then | |
| echo '.circleci/continue_config.yml' > /tmp/continuation-config-path.txt | |
| else | |
| echo '.circleci/continue_config_light.yml' > /tmp/continuation-config-path.txt | |
| fi | |
| set +e | |
| ./scripts/ci/has-code-changes.sh | |
| result=$? | |
| set -e | |
| if [ $result -eq 0 ]; then | |
| echo '.circleci/continue_config.yml' > /tmp/continuation-config-path.txt | |
| elif [ $result -eq 1 ]; then | |
| echo '.circleci/continue_config_light.yml' > /tmp/continuation-config-path.txt | |
| else | |
| echo "Error detecting code changes (exit $result)" >&2 | |
| exit 1 | |
| fi | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
| func canonicalAgent(name string) Agent { | ||
| switch Agent(name) { | ||
| case "github-copilot-cli": | ||
| if Agent(name) == "github-copilot-cli" { | ||
| return AgentGitHubCopilot | ||
| default: | ||
| return Agent(name) | ||
| } | ||
| return Agent(name) | ||
| } |
There was a problem hiding this comment.
some changes such as this one I believe is not necessary in this PR
There was a problem hiding this comment.
yes this is necessary because linter was failing
| defer func(r *os.File) { | ||
| _ = r.Close() | ||
| }(r) | ||
| defer func(w *os.File) { | ||
| _ = w.Close() | ||
| }(w) |
There was a problem hiding this comment.
Why was this change needed?
| BASE_REF="${1:-${BASE_REF:-origin/main}}" | ||
|
|
||
| if ! git rev-parse --verify "$BASE_REF" >/dev/null 2>&1; then | ||
| echo "list-changed-files: unknown base ref: $BASE_REF" >&2 | ||
| exit 2 | ||
| fi | ||
|
|
||
| MERGE_BASE="$(git merge-base HEAD "$BASE_REF")" | ||
| git diff --name-only "${MERGE_BASE}...HEAD" |
There was a problem hiding this comment.
can probably reuse what is being used in has-code-changes.sh
| set -euo pipefail | ||
|
|
||
| SCRIPT_DIR="$(CDPATH= cd -- "$(dirname -- "$0")" && pwd)" | ||
| ALLOWED='(\.md|\.svg|\.jpg)$' |
There was a problem hiding this comment.
Also, instead of maintaining this separate list, can we just make an "opposite" of the patterns we have in the other file? (scripts/ci/code-change-pathspecs.txt)
robertolopezlopez
force-pushed
the
feat/CLI-1625
branch
from
75e5ac5 to
80ad6e1
Compare
PR Reviewer Guide 🔍
|
2 participants
User description
Pull Request Submission Checklist
are release-note ready, emphasizing
what was changed, not how.
What does this PR do?
It skips the
test_and_releasepipeline for changes which do not touch code nor dependencies. Keeps the previous behaviour for others.Where should the reviewer start?
How should this be manually tested?
Right now the pipeline is running full in this PR.
After merge to main, there will be another PR showcasing the "light scenario".
What's the product update that needs to be communicated to CLI users?
PR Type
Enhancement
Description
Detect code changes vs. docs-only PRs.
Conditionally run full or lightweight CI.
Document new CI workflow behavior.
Diagram Walkthrough
flowchart LR A[CI Trigger] --> B{Detect Code Changes}; B -- Code Changes Found --> C[Full CI Pipeline]; B -- Docs-Only Changes --> D[Lightweight CI Pipeline];File Walkthrough
7 files
Changes required by linter, also failing in main branchNew script to detect code changesNew script to identify documentation-only changesHelper script to list changed filesImplement CI job for detecting code changesNew file for full CI pipeline configurationNew file for lightweight CI pipeline configuration1 files
Changes required by linter, also failing in main branch2 files
Tests for code change detection scriptTests for documentation-only change detection1 files
Document new CI pipeline behavior and logic1 files
Define pathspecs for code changes