"docs/vscode:/vscode.git/clone" did not exist on "bd91a1750e02c9cf789b7c0335f230d87e9615d9"
Unverified Commit c8f7ce90 authored by Keiven C's avatar Keiven C Committed by GitHub
Browse files

docs: add PR rebase guidelines and CI failure reproduction tips (#7298)


Signed-off-by: default avatarKeiven Chang <keivenchang@users.noreply.github.com>
Co-authored-by: default avatarKeiven Chang <keivenchang@users.noreply.github.com>
parent 9f01dd25
# PR Workflow Guidelines
Conventions for keeping pull requests healthy and reviewable.
## Keep Your Branch Close to Main
Be aware of PRs targeting `main` that are more than ~25 commits behind.
Stacked PRs targeting another branch are exempt.
### 1. Slower CI builds
CI builds start from a base image matching a recent `main` commit. The closer
your branch is to `main`, the more cache hits you get. We use BuildKit with
remote cache (`--cache-from`), but BuildKit layers are still content-addressed
-- if a `COPY` brings in source that changed since your branch point, that
layer and everything downstream rebuilds regardless. When your branch is far
behind:
- **Docker layer cache misses**: Changed source or Dockerfile instructions
since your branch point invalidate cached layers, forcing rebuilds
downstream.
- **Rust compilation cache misses**: More crate sources differ from what's
cached, so more crates recompile from scratch.
- **Dependency drift**: `Cargo.lock` and `requirements.txt` evolve on `main`.
Stale branches pull older versions that aren't cached, triggering fresh
downloads and builds.
A branch too far behind `main` can trigger a near-cold build that takes
45-60 minutes. A branch close to `main` reuses most of the cache and builds
in a fraction of that time.
### 2. Reviewer burden
Merge commits from main pollute the diff with unrelated changes. Rebasing
produces a clean, linear history where every commit is the PR author's.
```bash
# BAD -- merge pulls in unrelated files; reviewer has to filter them out
git merge main
# GOOD -- clean diff showing only your work
git fetch origin && git rebase origin/main
```
### Guidance
- **Rebase when you are more than ~25 commits behind main.**
- **Prefer `rebase` over `merge`** for linear history. Force-push after
rebasing (`git push --force-with-lease`).
- **Before requesting review**, check your distance from main:
```bash
git fetch origin
git rev-list --count HEAD..origin/main
```
More than 25? Rebase first.
- **If CI is slow**, check your base commit age before debugging other causes.
- **Stacked PRs** are exempt. If a PR targets another branch in a stack,
distance from `main` is expected and not a problem until the stack lands.
...@@ -8,6 +8,27 @@ Rules and conventions for Python tests in this repository. ...@@ -8,6 +8,27 @@ Rules and conventions for Python tests in this repository.
wastes shared GPU minutes and blocks other PRs. A local run catches most wastes shared GPU minutes and blocks other PRs. A local run catches most
failures in seconds. failures in seconds.
### Reproducing CI failures locally
Don't guess from log snippets. Pull the same container image CI built and
reproduce the failure locally:
```bash
# 1. Find the image:tag in the CI job's "docker build" or "docker push" step.
# 2. Run it with GPU access:
docker run --rm -it --gpus all <ci-image>:<tag> bash
# 3. Run the failing test:
python3 -m pytest -xvv tests/path/to/test_that_failed.py::test_name
```
If the test passes locally in the CI container, the failure is likely a
resource or timing issue in CI. If it fails, you have an exact reproduction.
Pull the container, reproduce, fix, verify -- in that order. Don't make
speculative fixes from logs.
Always use the venv-aware invocation -- never bare `pytest`: Always use the venv-aware invocation -- never bare `pytest`:
```bash ```bash
......
...@@ -28,6 +28,11 @@ reviews: ...@@ -28,6 +28,11 @@ reviews:
unit_tests: unit_tests:
enabled: false enabled: false
path_instructions: path_instructions:
- path: "**"
instructions: |
Flag PRs more than ~25 commits behind main. Recommend rebasing.
Exception: stacked PRs targeting another branch are exempt.
See .ai/pr-workflow-guidelines.md.
- path: "**/*.py" - path: "**/*.py"
instructions: "Review against .ai/python-guidelines.md" instructions: "Review against .ai/python-guidelines.md"
- path: "**/tests/**/*.py" - path: "**/tests/**/*.py"
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment