Skip to content

Commit eb2936c

Browse files
Bueller87Bueller87
committed
ci: add PR template, guidance, and gitar rule for quality checks
- Add .github/pull_request_template.md with sections for sample area, what changed, why, testing, risks, release notes, and docs - Add .github/pull_request_guidance.md with samples-specific guidance and examples (Gradle commands, N/A when appropriate) - Add .gitar/rules/pr-description-quality.md to evaluate PR descriptions against the guidance Signed-off-by: “Kevin” <kevlar_ksb@yahoo.com>
1 parent 25b69fa commit eb2936c

3 files changed

Lines changed: 149 additions & 0 deletions

File tree

‎.gitar/rules/pr-description-quality.md‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
---
2+
title: PR Description Quality Standards
3+
description: Ensures PR descriptions meet quality criteria for the Java Samples repo using guidance from the PR template and .github/pull_request_guidance.md
4+
when: PR description is created or updated
5+
actions: Read PR template and guidance, then report requirement status
6+
---
7+
8+
# PR Description Quality Standards
9+
10+
When evaluating a pull request description:
11+
12+
1. **Read the PR template guidance** at `.github/pull_request_guidance.md` to understand the expected guidance for each section
13+
2. Apply that guidance to evaluate the current PR description
14+
3. Provide recommendations for how to improve the description.
15+
16+
## Core Principle: Why Not How
17+
18+
From https://cbea.ms/git-commit/#why-not-how:
19+
- **"A diff shows WHAT changed, but only the description can explain WHY"**
20+
- Focus on: the problem being solved, the reasoning behind the solution, context
21+
- The code itself documents HOW - the PR description documents WHY
22+
23+
## Evaluation Criteria
24+
25+
### Required Sections (must exist with substantive content per PR template guidance)
26+
27+
1. **Which sample(s) or area?** (optional but recommended)
28+
- One line listing area(s) touched: hello, fileprocessing, spring, shadowing, calculation, bookingsaga, replaytests, common, README, build
29+
- Helps reviewers route; skip flagging if area is obvious from paths
30+
31+
2. **What changed?**
32+
- 1-2 line summary of WHAT changed technically
33+
- Focus on key modification, not implementation details
34+
- Link to GitHub issue encouraged for non-trivial changes; optional for trivial doc/sample tweaks
35+
- Template has good/bad examples
36+
37+
3. **Why?**
38+
- Context and motivation (why not how)
39+
- Enough rationale for reviewers to understand the goal (e.g. improving clarity, fixing compatibility, aligning with docs)
40+
- Must explain WHY this approach was chosen
41+
42+
4. **How did you test it?**
43+
- Concrete, copyable commands with exact invocations
44+
- GOOD: `./gradlew build`, `./gradlew test`, or `./gradlew -q execute -PmainClass=com.uber.cadence.samples.hello.HelloActivity`
45+
- BAD: "Tested locally" or "See tests"
46+
- Expect Gradle and/or sample execution commands; no canary or integration server setup required
47+
48+
5. **Potential risks**
49+
- Often N/A for sample-only or doc-only changes
50+
- Call out when relevant: dependency upgrades, behavior changes for someone copying the sample, build/config changes
51+
- Don't require lengthy text when N/A is appropriate
52+
53+
6. **Release notes**
54+
- Optional for this repo. Use when change is user-facing (e.g. new sample, notable README change)
55+
- Can be N/A for internal refactors or tiny fixes
56+
- Don't require lengthy text when N/A is appropriate
57+
58+
7. **Documentation Changes**
59+
- Often relevant when adding or changing samples (main README, sample READMEs, links to cadence or cadence-docs)
60+
- Only mark N/A if certain no docs are affected
61+
62+
### Quality Checks
63+
64+
- **Skip obvious things** - Don't flag items clear from folder structure (e.g. area from paths)
65+
- **Skip trivial refactors** - Minor formatting/style changes don't need deep rationale
66+
- **Don't check automated items** - CI, linting are automated
67+
68+
## FORBIDDEN - Never Include
69+
70+
- "Issues Found", "Testing Evidence Quality", "Documentation Reasoning", "Summary" sections
71+
- "Note:" paragraphs or explanatory text outside recommendations
72+
- Grouping recommendations by type
73+
74+
## Section Names (Use EXACT Brackets)
75+
76+
- **[Which sample(s) or area?]**
77+
- **[What changed?]**
78+
- **[Why?]**
79+
- **[How did you test it?]**
80+
- **[Potential risks]**
81+
- **[Release notes]**
82+
- **[Documentation Changes]**

‎.github/pull_request_guidance.md‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
<!-- List the area(s) touched so reviewers know where to look.
2+
Examples: hello, fileprocessing, spring, shadowing, calculation, bookingsaga, replaytests, common, README, build -->
3+
**Which sample(s) or area?**
4+
5+
6+
<!-- 1-2 line summary of WHAT changed technically.
7+
- Link to a GitHub issue when applicable (encouraged for larger changes; optional for trivial doc/sample tweaks)
8+
- Good: "Added HelloActivityRetry sample demonstrating retry options" or "Updated README run instructions for Java 21"
9+
- Bad: "updated code" or "fixed stuff" -->
10+
**What changed?**
11+
12+
13+
<!-- Provide context and motivation (see https://cbea.ms/git-commit/#why-not-how). Focus on WHY, not how.
14+
- Lighter than core Cadence repo: e.g. "improving clarity for new users," "fixing sample broken on Java 21," "aligning with cadence-docs"
15+
- Still give enough rationale for reviewers to understand the goal
16+
- Good: "HelloActivity didn't show retry behavior; this sample demonstrates RetryOptions so users can copy-paste a working example."
17+
- Bad: "Improves samples" -->
18+
**Why?**
19+
20+
21+
<!-- Include concrete, copy-paste commands so another maintainer can reproduce your test steps.
22+
- Prefer: `./gradlew build`, `./gradlew test` (or a targeted test if relevant)
23+
- For runnable samples: `./gradlew -q execute -PmainClass=com.uber.cadence.samples.<area>.<Class>`
24+
- Good: Full commands reviewers can copy-paste to verify
25+
- Bad: "Tested locally" or "See tests" -->
26+
**How did you test it?**
27+
28+
29+
<!-- Often N/A for sample-only or doc-only changes. Call out when relevant:
30+
- Dependency upgrades (e.g. cadence-client version)
31+
- Behavior changes that could affect someone copying the sample
32+
- Build or config changes
33+
- If truly N/A, you can mark it as such -->
34+
**Potential risks**
35+
36+
37+
<!-- Optional for this repo. Use when the change is user-facing (e.g. new sample, notable README change).
38+
- Can be N/A for internal refactors, tiny fixes, or incremental work -->
39+
**Release notes**
40+
41+
42+
<!-- Did you update the main README or a sample README (e.g. src/main/java/.../shadowing/README.md)?
43+
- Any links to cadence or cadence-docs that need updating?
44+
- Only mark N/A if you're certain no docs are affected -->
45+
**Documentation Changes**
46+

‎.github/pull_request_template.md‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<!-- If you are new to contributing or want a refresher, please read ./pull_request_guidance.md -->
2+
**Which sample(s) or area?**
3+
4+
5+
**What changed?**
6+
7+
8+
**Why?**
9+
10+
11+
**How did you test it?**
12+
13+
14+
**Potential risks**
15+
16+
17+
**Release notes**
18+
19+
20+
**Documentation Changes**
21+

0 commit comments

Comments
 (0)