Skip to content

Commit b9cd522

Browse files
docs(adr): ADR-017 PowerShell output schema consistency (#302)
Documenting design decision to include all properties in PowerShell output objects with null/0 values rather than conditionally excluding them. This ensures consistent schemas for consumers. Addresses comment on PR #235 regarding IssueCommentCount property. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: rjmurillo[bot] <rjmurillo-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 93bbbec commit b9cd522

File tree

1 file changed

+117
-0
lines changed

1 file changed

+117
-0
lines changed
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
# ADR-017: PowerShell Output Schema Consistency
2+
3+
## Status
4+
5+
Accepted
6+
7+
## Date
8+
9+
2025-12-23
10+
11+
## Context
12+
13+
When designing PowerShell cmdlets and scripts that return structured data, a decision must be made about how to handle optional or conditional properties in output objects.
14+
15+
**Specific scenario**: `Get-PRReviewComments.ps1` has an `-IncludeIssueComments` switch. When this switch is NOT used, should the output object:
16+
17+
1. Include `IssueCommentCount` with value 0
18+
2. Exclude `IssueCommentCount` entirely from the output object
19+
20+
This pattern applies broadly to any PowerShell skill script that has optional data fields.
21+
22+
## Decision
23+
24+
**Include all properties in output objects with null/0 values when not populated, rather than conditionally excluding properties from the output schema.**
25+
26+
## Rationale
27+
28+
### Schema Consistency
29+
30+
Consumers can rely on properties always existing, simplifying deserialization and property access:
31+
32+
```powershell
33+
# With consistent schema - always works
34+
$result.IssueCommentCount
35+
36+
# With conditional schema - must check first
37+
if ($result.PSObject.Properties['IssueCommentCount']) {
38+
$result.IssueCommentCount
39+
}
40+
```
41+
42+
### Backward Compatibility
43+
44+
Adding properties to existing output objects is generally safe. However, if consumers use strict schema validators (e.g., JSON Schema, Pester assertions on exact properties), having variable schemas causes brittleness.
45+
46+
Including all properties from the start, even when null/0, prevents future breaking changes.
47+
48+
### PowerShell Cmdlet Conventions
49+
50+
Common PowerShell cmdlets follow this pattern:
51+
52+
| Cmdlet | Behavior |
53+
|--------|----------|
54+
| `Get-ChildItem` | Always includes Size, LastWriteTime even for directories |
55+
| `Get-Process` | Always includes WorkingSet, CPU even if 0 |
56+
| `Get-Service` | Always includes DependentServices even if empty array |
57+
58+
### Pipeline Friendliness
59+
60+
PowerShell pipelines work best with consistent object shapes:
61+
62+
```powershell
63+
# Works reliably with consistent schema
64+
Get-PRReviewComments | Select-Object ReviewCommentCount, IssueCommentCount | Export-Csv
65+
66+
# Fails unpredictably with variable schema
67+
Get-PRReviewComments | Select-Object ReviewCommentCount, IssueCommentCount # Error if property missing
68+
```
69+
70+
### Alternatives Considered
71+
72+
#### Conditional Property Inclusion
73+
74+
```powershell
75+
$output = [PSCustomObject]@{
76+
ReviewCommentCount = $reviewCount
77+
}
78+
if ($IncludeIssueComments) {
79+
$output | Add-Member -NotePropertyName IssueCommentCount -NotePropertyValue $issueCount
80+
}
81+
```
82+
83+
**Rejected because**: Creates variable schema that complicates consumption and breaks strict validators.
84+
85+
#### Separate Output Types
86+
87+
Create different output types for different switch combinations.
88+
89+
**Rejected because**: Explosive growth in types (2^n for n optional switches), harder to maintain and document.
90+
91+
## Consequences
92+
93+
### Positive
94+
95+
- Predictable API contracts for consumers
96+
- Simpler downstream code (no property existence checks)
97+
- Better pipeline compatibility
98+
- Future-proof when adding new optional features
99+
100+
### Negative
101+
102+
- Slightly more verbose output when properties are unused
103+
- May require documentation explaining that 0/null means "not requested" vs "none found"
104+
105+
## Compliance
106+
107+
All PowerShell skill scripts under `.claude/skills/` SHOULD follow this pattern:
108+
109+
1. Define complete output schema upfront
110+
2. Include all properties in every output object
111+
3. Use null/0/empty array for unpopulated optional fields
112+
4. Document which properties are conditional and what null/0 means
113+
114+
## References
115+
116+
- PR #235: `Get-PRReviewComments.ps1` implementation discussion
117+
- PowerShell cmdlet design best practices

0 commit comments

Comments
 (0)