Imported from BhaveshY/google-eng-practices-skill (
SKILL.md). Install upstream withnpx skills add BhaveshY/google-eng-practices-skill. Copyright stays with the author (CC-BY-3.0).
Google Engineering Practices
Use this skill when preparing, reviewing, responding to, or deciding on a code change. It adapts Google's public Engineering Practices into agent instructions; project ownership, style guides, security policy, and release process still win.
Attribution: adapted from Google Engineering Practices, https://github.com/google/eng-practices, commit 3bb3ec25b3b0199f4940b1aa75f0ac5c5753301c, licensed CC-BY 3.0.
Core Standard
Approve once the CL improves overall code health, even if it is not perfect. Do not approve code-health regressions except for a real emergency. Prefer technical facts, data, style guides, and local consistency over personal preference.
Code health means the change leaves the system easier to understand, maintain, test, operate, or safely evolve. Forward progress matters, but "clean it up later" is weak when the CL introduces new complexity.
Triggers
Use this workflow for:
- Author help: draft or improve a PR/CL description, split a large change, prepare evidence for review, respond to comments.
- Reviewer help: inspect a diff, generate review findings, classify comments, decide whether to approve or request changes.
- Review process help: resolve disagreement, handle slow review, label nits, record partial review scope, evaluate emergency exceptions.
For local diffs, run scripts/google_eng_practices_check.py from this skill package inside the target git repo before final review output when practical.
Author Workflow
Before asking for review:
- Read the diff and state the one self-contained purpose of the CL. If there are multiple purposes, suggest a split.
- Write a first line that says specifically what the CL does. Use an imperative sentence and keep it skimmable.
- Add body context: why the change exists, important tradeoffs, migration or rollback notes, tests run, risks, and links or bug IDs with enough local context to survive link rot.
- Keep related production and test changes together. For behavior changes, add or update tests in the same CL unless this is a true emergency.
- Separate large refactors, formatting-only churn, generated updates, dependency updates, and feature logic when they would make review harder.
- If a reviewer says code is unclear, first clarify the code. Use a code comment only for reasoning the code cannot express. Do not rely on review-thread explanations for future readers.
Use templates/pr-description.md for author-facing output.
Reviewer Workflow
Before reading files:
- Identify the CL's stated purpose and whether the description explains what and why.
- Inspect size, file spread, generated/vendor files, deletions, tests, docs, and formatting churn.
- Decide whether the change is reviewable as one CL. If it is too large or mixed-purpose, ask for a split before spending time on line-by-line review.
Review sequence:
- Broad view: should this change happen in this codebase now? If not, respond quickly with the technical reason and a suggested path.
- Main path first: review the central design and highest-risk files before smaller follow-up edits.
- Every assigned line: review human-written code line by line. For generated code, large data, or files outside your scope, state what you did and did not review.
- Context pass: read surrounding code where the diff alone is not enough to judge design, complexity, or consistency.
- Evidence pass: check tests, docs, observability, migration, rollback, and user/developer impact.
Review focus areas:
- Design: correct ownership, integration, abstraction boundaries, timing, and whether the feature belongs here.
- Functionality: intended behavior, user impact, edge cases, concurrency, error handling, security/privacy/accessibility/i18n when relevant.
- Complexity: line/function/class/system complexity, over-engineering, unused future-proofing, harder modification paths.
- Tests: meaningful assertions, negative and edge cases, maintainability, same-CL coverage for changed behavior.
- Naming: names communicate purpose without needless length.
- Comments: comments explain why, not obvious what; documentation explains purpose, usage, and behavior.
- Style/consistency: follow authoritative style guide; otherwise prefer local consistency if it does not worsen code health.
- Documentation: update docs when build/test/use/release/API/CLI/config behavior changes, and remove docs for deleted/deprecated behavior.
Comment Taxonomy
Write comments about code, not the person. Make each comment actionable: observation, impact, requested change or acceptable options.
Labels:
Blocker:must be resolved before approval because it affects correctness, security, data loss, build/test reliability, API contract, or code health.Required:must be resolved before approval because it materially improves maintainability, clarity, tests, docs, or design.Consider:optional suggestion with a reason; author may choose another defensible approach.Nit:polish, style, typo, or minor consistency point; never block on nits alone.FYI:information for future work; no action expected in this CL.
Prefer questions when you genuinely need information. Prefer direct requests when the requirement is clear. If the author's explanation belongs in code or docs, ask them to move it there.
Use templates/review-comment.md for reusable comment formats.
Decision Rules
Approve when:
- The CL improves code health overall.
- Required comments are addressed or can be safely handled as trusted follow-ups.
- Remaining comments are optional, nits, or clearly documented "LGTM with comments" items.
- You understand the assigned human-written code or have scoped your approval to the parts you reviewed.
Request changes when:
- The change worsens code health, correctness, security, privacy, data integrity, or operability.
- Tests or docs are materially missing for the changed behavior.
- The design or complexity risk needs rework before merge.
- The CL is too large or mixed-purpose to review reliably.
- Generated/vendor output is being rubber-stamped without a reviewed source, tool, or verification plan.
Comment without approval when:
- You need clarification, a broader owner, a specialist reviewer, or a design direction before deciding.
- You reviewed only part of the CL and cannot approve the rest.
Use templates/review-decision.md for approve/request-changes/comment templates.
Conflict Handling
Start with consensus through technical facts and code health. If the author disagrees, consider whether they are right or closer to the implementation details. If they have a sound argument, drop the issue.
When you still disagree, explain the code-health impact and the tradeoff you are asking for. Do not let a CL sit indefinitely. Move contentious back-and-forth to synchronous discussion, then record the outcome in the review. Escalate to maintainers, technical leads, team discussion, or engineering management when consensus stalls.
Emergency Exception
A real emergency is a small change that fixes an acute production/user/legal/security launch-blocking issue where speed of the whole review matters. Soft deadlines, Friday timing, timezone gaps, long-running feature pressure, or ordinary rollbacks are not emergencies.
In an emergency, prioritize correctness of the emergency fix and fast review. Afterward, schedule a normal review pass and cleanup. Do not use emergency process as a general reason to bypass code health.
Anti-Patterns To Call Out
- Giant mixed-purpose CLs combining feature logic, refactor, formatting, generated output, and dependency changes.
- Blocking on personal style preferences without a style-guide or local-consistency basis.
- Approving code that clearly worsens code health because a follow-up might happen later.
- Rubber-stamping generated code without reviewing generator inputs, tool trust, or verification evidence.
- Comments that only say "fix this" without reason or direction.
- Perfectionism that blocks a beneficial CL over non-critical polish.
- Review-thread explanations that should be code clarity, code comments, docs, or tests.
Expected Output
For author prep, output:
- Revised PR/CL description.
- Split recommendation if needed.
- Missing tests/docs/risk evidence.
- Suggested reviewer notes.
For review, output findings first, ordered by severity, each with file/line when available and a labeled comment. Then include open questions, decision, and concise review summary. If no issues are found, say that clearly and note residual risk such as unrun tests or unreviewed generated files.
References
references/source-map.md: source files behind each tactic and attribution details.templates/pr-description.md: author template.templates/review-comment.md: reviewer comment template.templates/review-decision.md: decision templates.scripts/google_eng_practices_check.py: dependency-free git diff checklist.