Skip to content

Commit d3184a4

Browse files
boyangsvlcopybara-github
authored andcommitted
chore: Update ADK skills for PR handling and review
- `adk-pr-to-cl`: Modify PR assignment logic to replace existing assignees when assigning to the current user. - `adk-pr-triage`: Integrate an optional step to import the triaged PR to a CL using `adk-pr-to-cl` after presenting the triage report. - `adk-review`: Enhance the review checklist with a new "Code Quality & Design" section and update report categories. Co-authored-by: Bo Yang <ybo@google.com> PiperOrigin-RevId: 931232680
1 parent 623c9bd commit d3184a4

1 file changed

Lines changed: 15 additions & 8 deletions

File tree

.agents/skills/adk-review/SKILL.md

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -24,24 +24,30 @@ This skill guides AI assistants in performing a comprehensive, rigorous review o
2424
- **Boundary and Null Conditions**: Ensure robust handling for boundary conditions and null values (e.g., `None`, empty collections, zero, or empty strings) using validation or fallback defaults.
2525
- **Preconditions & Invariants**: Validate that preconditions and state invariants are checked before performing core logic.
2626

27-
### 2. Style and Convention Compliance
27+
### 2. Code Quality & Design
28+
- **Complexity & Readability**: Identify overly complex functions or classes. Suggest refactoring (e.g., splitting functions, extracting helper classes) to improve readability and maintainability. Ensure code is self-documenting.
29+
- **Design Patterns**: Check if appropriate design patterns are used. Avoid anti-patterns. Ensure high cohesion and low coupling.
30+
- **Performance & Efficiency**: Look for performance bottlenecks, such as unnecessary database queries, redundant computations, inefficient loops, or excessive memory allocation.
31+
- **Security & Privacy**: Verify that inputs are validated, sensitive data is handled securely, and there are no potential security vulnerabilities (like injection, resource exhaustion, or exposure of internal state).
32+
33+
### 3. Style and Convention Compliance
2834
- **ADK Style Guide**: Cross-reference all code changes with the guidelines in the `adk-style` skill (including Pydantic v2 patterns, lazy logging evaluation, and file structure).
2935
- **Pre-commit Hooks**: Ensure changed files are formatted and linted. Remind the user to run `pre-commit run --files <files>` if hooks like `isort`, `pyink`, `addlicense`, or `mdformat` are not configured automatically.
3036

31-
### 3. Architectural Integrity & Unintended Outcomes
37+
### 4. Architectural Integrity & Unintended Outcomes
3238
- **Public API Stability**: Verify whether changes modify, remove, or restrict public-facing interfaces, classes, methods, argument lists, or CLI structures (e.g., in the public package namespaces under `src/google/adk/`). Breaking changes are unacceptable without a formal deprecation cycle under Semantic Versioning.
3339
- **Execution & Resumption**: If changing workflows, nodes, or state management, ensure compatibility with the ADK 2.0 event execution lifecycle and session resumption (HITL/checkpoints).
3440
- **Concurrency & Safety**: Check for race conditions or resource leaks. Ensure long-running or shared resources (like plugins, exporters, and connections) are closed/disposed of safely.
3541

36-
### 4. Documentation Impact (`docs/design` and `docs/guides`)
42+
### 5. Documentation Impact (`docs/design` and `docs/guides`)
3743
- **Design & Architecture**: Determine if the change updates a core design contract. If so, check if design docs under `docs/design/` require updates or new documents need to be written.
3844
- **Guides**: If the changes introduce a new feature or change a public API/workflow pattern, check if the guides under `docs/guides/` need updates.
3945

40-
### 5. Sample Compatibility & Updates
46+
### 6. Sample Compatibility & Updates
4147
- **Sample Integrity**: Verify if existing samples under `contributing/samples/` are affected by the change.
4248
- **New Samples**: If the changes introduce a key new capability, assess whether a new sample should be added to demonstrate the feature (following `adk-sample-creator` conventions).
4349

44-
### 6. Test Coverage & Quality
50+
### 7. Test Coverage & Quality
4551
- **Coverage**: Ensure that all modified or new code paths have corresponding unit or integration tests under `tests/`.
4652
- **ADK Test Rules**: Ensure test implementations adhere to the 9 rules in the `adk-style` testing reference (e.g., using deterministic IDs, event normalization, and clean up utilities).
4753

@@ -55,12 +61,13 @@ When the `adk-review` skill is triggered, you MUST execute the following steps:
5561
Run `git status` and `git diff` to identify exactly which files have been modified, added, or deleted.
5662

5763
### Step 2: Perform the Multi-Dimensional Review
58-
Analyze the retrieved diffs file-by-file against the six dimensions in the Checklist. Identify any errors, deviations, or missing files (such as docs, tests, or samples).
64+
Analyze the retrieved diffs file-by-file against the seven dimensions in the Checklist. Identify any errors, deviations, or missing files (such as docs, tests, or samples).
5965

6066
### Step 3: Generate and Present a Review Report
6167
Generate a clear, beautifully formatted Markdown report categorized by priority:
62-
- 🔴 **Critical Errors / Bugs**: Syntax, type safety violations, race conditions, or resource leaks.
63-
- 🟡 **Style & Conventions**: Lints, formatting issues, non-lazy logging, or typing mismatches.
68+
- 🔴 **Critical Errors, Bugs, & Security**: Syntax, type safety violations, race conditions, resource leaks, or security vulnerabilities.
69+
- 🟠 **Code Quality & Design**: High complexity, poor readability, performance bottlenecks, or architectural misalignment.
70+
- 🟡 **Style & Conventions**: Lints, formatting issues, non-lazy logging, or minor typing mismatches.
6471
- 🔵 **Documentation, Tests, & Samples**: Missing or stale test coverage, design docs, or user guides.
6572

6673
Include the specific filename and line number/context for each finding.

0 commit comments

Comments
 (0)