github-pr-description-writer

name	github-pr-description-writer
description	Guidelines for generating clear, concise, and objective GitHub Pull Request descriptions.

Writing GitHub Pull Request Descriptions

This skill provides guidelines for creating clear, concise, and objective pull request descriptions for the ConnectedHomeIP repository. The goal is to make PRs easy for human maintainers to understand and review.

General Rules

These rules apply globally to all sections of the PR description.

Style and Tone

• Objective & Technical: Write for a technical audience. Use precise technical terms.
• Concise: Keep descriptions brief but informative. Avoid walls of text. Use bullet points where appropriate.
• No Prose or Hype: Do not use marketing-style language, emojis, or subjective/hyperbolic terms (e.g., "amazing", "triumph", "perfect", "flawless").
• No Filler: Avoid conversational filler. Get straight to the point.

Noise Reduction Rule

• Say Nothing if Nothing to Say: Do not include sections or bullet points with "None" or "N/A". If a section (like Caveats or #### Related issues) has no content, omit the heading and content entirely to reduce noise. Exception: The #### Testing section must always be present; you may use N/A there for trivial changes if accompanied by a brief justification.

Keeping in Sync Rule

• Update on Change: The PR description MUST be updated after every commit if the content of the PR materially changes.
  • Requires Update:
    • Adding or deleting a test.
    • Changing the implementation approach (e.g., swapping a library function or logic).
    • Adding or removing a feature flag.
  • Does NOT Require Update:
    • Rebasing or merging with master.
    • Commit message wording changes only.
    • Formatting-only changes that do not alter logic.
    • Minor test execution adjustments (e.g., changing log levels in test run) if they don't change what was tested.

Formatting Guidelines

The PR description must follow a structured format based on the repository's template, focusing on: Summary, Related Issues, and Testing.

1. Summary Section (#### Summary)

The summary should explain the "what", "why", and "how" of the changes. It must use the following sub-headings (where ##### Impact is optional):

Problem

• Explain the underlying issue that this PR addresses. What is broken, missing, or needs improvement?
• Optimize for Quick Review: Use formatting that allows reviewers to instantly grasp the problem:
  • Structured Breakdown: Use nested bullets to decompose complex problems (e.g., describing a pattern, then listing its consequences like flakiness or inefficiency).
  • Bolding for Emphasis: Highlight key trade-offs, states, or impacts using bold text.
  • ASCII Diagrams: If the problem involves a workflow, state machine, or timing sequence, include a brief ASCII diagram to visualize it.
  • If the consequences/impact are clearly explained within this structured breakdown, the ##### Impact section should be omitted to reduce noise.

Impact

• Explain why this is a problem if it is not immediately obvious. What are the consequences of not fixing it? Omit if already covered in the structured breakdown of the ##### Problem section.

Solution

• Explain the proposed solution and why it fixes the problem.
• Holistic & High-Level: Focus on the new capabilities, public APIs, or architectural changes. Do NOT list individual files, classes, or internal implementation details (e.g., "Refactored MockX to use dataclass") unless they are critically important to the architecture. The diff itself shows the file-level changes; the PR description should show the big picture.
• ASCII Diagrams: If the solution introduces a new workflow, state machine, or architecture, consider including a brief ASCII diagram to visualize the new flow.

Caveats (Optional)

• Document any limitations, side effects, performance/memory impacts, or specific concerns that reviewers should be aware of. Omit this section entirely if there are no caveats.

2. Related Issues Section (#### Related issues)

• Reference any related GitHub issues.
• Use closing keywords (e.g., Fixes #12345) if the PR should close the issue upon merging.

3. Testing Section (#### Testing)

Every PR must document how the changes were tested.

• Automated Tests:
  • If unit tests were added/updated, state so (e.g., Added unit tests in src/app/tests/...).
  • If integration tests (Python/YAML) were used, specify the test names (e.g., Verified with TC_S_2_3.py).
• Manual Testing:
  • If manual testing was performed, provide the exact commands and steps to reproduce.
  • Explain why automated testing was not possible.
• Trivial Changes:
  • If the change is trivial (e.g., documentation typo), it still requires a #### Testing section. You may use N/A with a brief justification.

---

Example PR Description

Here is an example of a good PR description:

#### Summary

Fixes a crash in the DNS-SD client when processing malformed SRV responses.

##### Problem

-   The DNS-SD client does not validate the target host name length in incoming
    SRV records before copying it to a fixed-size buffer.

##### Impact

-   A malformed SRV record with a target host name longer than 63 characters
    causes a buffer overflow, leading to a crash.

##### Solution

-   Added validation to ensure the host name length does not exceed
    `kMaxHostNameLength` before copying.
-   Malformed records are now safely discarded and a warning is logged.

##### Caveats

-   This change discards malformed records and logs a warning.

#### Related issues

Fixes #72327

#### Testing

-   Added unit test `TestDnssdClient_MalformedSrvRecord` in
    `src/lib/dnssd/tests/TestDnssdImpl.cpp` to verify that malformed records are
    discarded and do not cause a crash.

Example 2: Structured Breakdown & Holistic Solution

Here is an example using a structured breakdown with bold highlights for the problem, and a high-level solution summary:

#### Summary

Implemented `WaitForAttributeValue` command in YAML runner to allow tests to
wait for an attribute to reach a specific value, improving test robustness and
reducing execution time. Supported in all Python-based runners.

##### Problem

-   YAML tests lacked polling, requiring a fragile **"trigger transition → sleep
    → verify"** pattern.
-   This fixed-delay pattern is:
    -   **Racy**: Leads to flakiness in slow environments (like CI) if
        transitions take longer than the sleep duration.
    -   **Inefficient**: Forces fast runs to wait for the maximum padded
        duration even if the transition finishes early.

##### Solution

-   Added support for the `WaitForAttributeValue` command to wait for an
    attribute to reach a target value within a timeout, avoiding fixed sleeps.
-   Added the underlying infrastructure to support this across all Python-based
    runners (`chip-tool`, `darwin-framework-tool`, and the REPL runner).
-   Made the extra timeout buffer (slop) configurable (defaults to 250ms
    locally, but configured to 2000ms in GitHub Actions workflows to prevent
    flakiness on slow CI runners).
-   Included a polling history (last 10 attempts with values/errors) in the
    timeout exception message to improve debuggability when a wait fails.

##### Caveats

-   The command uses active polling (100ms interval) to check the attribute
    value.

#### Testing

-   Added integration test
    `src/app/tests/suites/Test_WaitForAttributeValue.yaml` to verify the
    command's timeout behavior.
-   Registered the new test in `src/app/tests/suites/ciTests.json` to enable it
    in CI.
-   Migrated Step 5b of `src/app/tests/suites/certification/Test_TC_S_2_3.yaml`
    to use `WaitForAttributeValue` and verified it passes successfully.