Amdsmi Commit And Pr Conventions

Commit & PR Conventions — amd-smi

The single source of truth for what a commit message and a pull request look like in amd-smi. amdsmi-restructure-commits triggers this skill for message wording; amdsmi-changelog-automation owns CHANGELOG.md entries.

Core principle: Write for an expert who already knows the codebase. Bullets, not prose. State what changed and why — never narrate how the code works.

Title Convention (commits AND PRs)

type(amdsmi): imperative summary

The PR bot (tools/systems_pr_bot) enforces Conventional Commits on PR titles. Use the same form for commit subjects so a single-commit / squash PR auto-titles correctly.

Good: fix(amdsmi): reject nullptr mode pointer in compute-partition getter Bad: [AMD-SMI] Fixed the partition bug. (legacy tag, past tense, period)

No [AMD-SMI] / [ROCM-NNNNN] tag — the […] form fails the bot's title regex. Ticket references live only in the PR's JIRA ID section (below).

Self-check a subject (structure + length) before committing:

SUBJECT="fix(amdsmi): reject nullptr mode pointer in compute-partition getter"
printf '%s' "$SUBJECT" | grep -qP '^(feat|fix|docs|style|refactor|perf|test|build|ci|chore|revert)(\([a-z][a-z0-9_/-]*\))?!?: .+[^.]$' \
  && [ "${#SUBJECT}" -le 72 ] && echo OK || echo 'FAIL: not type(scope): imperative, or ends in a period, or >72 chars'

The regex enforces the type list, a lowercase scope, no trailing period, and a body after the colon; the length test enforces the ≤ 72 cap. It cannot judge mood — a subject can pass the regex and still be past-tense or a noun phrase, so read the verb by eye (see the Mood row).

Commit Message Body

type(amdsmi): imperative summary

- Verb-first bullet: what changed and why it was needed
- One bullet per distinct logical change
- Group by file/layer when a commit spans the cascade

Signed-off-by: Full Name <email@amd.com>

Rules:

• Blank line after the subject; wrap body at 72 columns.
• Each bullet starts with a verb (Add, Fix, Remove, Refactor, Implement).
• Include the "why" only when it is not self-evident from the change.
• No JIRA ticket references in the commit body. Reference the code/behavior, not ROCM-NNNNN. Tickets live in the PR's JIRA ID section only.
• No internal labels ("regression guard", sprint names) in the message.
• Signed-off-by is required (git commit -s); preserve original authors and add Co-authored-by: trailers when squashing multiple authors.
• If a single bullet needs a full paragraph to explain, the change is probably too large or the root cause is unclear — stop and reconsider the split.

Pull Request Body

Follow the rocm-systems template (.github/pull_request_template.md) — these exact sections, in order:

House style for ## Technical Details (matches the project's best PRs):

## Technical Details

**Header / docs** (`include/amd_smi/amdsmi.h`):
- Corrected the `@retval` list — setter returns `AMDSMI_STATUS_NO_PERM`, not `_PERMISSION`

**Public API** (`src/amd_smi/amd_smi.cc`):
- Getter now rejects a `nullptr` mode pointer with `AMDSMI_STATUS_INVAL`

Rules:

• Bullets, not paragraphs. No ticket numbers outside JIRA ID.
• Assume the reviewer is an amd-smi developer — skip background explanations.
• Don't restate the diff line-by-line; summarize each logical change once.
• Mention the cascade layers touched (header → impl → wrapper → interface → CLI → docs) so reviewers can check coverage.

PR Bot Policy Gate

Every PR to develop is checked by the Systems PR Bot (tools/systems_pr_bot). Two failures add the "Not ready to Review" label and block the PR; the rest are advisory rows in the bot's results comment.

A source change with no accompanying test is the most common block — pair every code change with a test, or split a docs-only PR out, before opening. Separately, pre-commit is a required CI status check (must be green to merge) but is not part of the bot's label logic above.

Brevity Caps

Common Mistakes