Skip to content

Commit 12c1947

Browse files
authored
Overhaul XML documentation for ObservableCacheEx extension methods (#1080)
* docs: overhaul XML comments for ObservableCacheEx extension methods Add uniform event behavior tables (Add/Update/Remove/Refresh/OnError/OnCompleted), seealso cross-references, inheritdoc for secondary overloads, and 'Worth noting' sections across all 3 ObservableCacheEx partial class files (~306 overloads). Key improvements: - 77 event behavior tables describing exact per-change-reason behavior - 129 seealso cross-references linking related operators - 107 inheritdoc tags for secondary overloads with delta remarks - 36 'Worth noting' sections calling out non-obvious behavior - All old-style generic param descriptions replaced with specific text - Summaries trimmed to 1-3 sentences, detail moved to remarks * docs: flesh out MergeChangeSets and MergeManyChangeSets documentation MergeManyChangeSets now has separate parent-side and child-side event tables, mirroring the join operator documentation style. Describes parent Add/Update/ Remove effects on child subscriptions, and child Add/Update/Remove/Refresh effects on downstream with conflict resolution details. MergeChangeSets adds overload family guide explaining the three axes: source type (dynamic/pair/static), conflict resolution (none/comparer/equality/both), and completion behavior (completable flag). * docs: add two-table structure to *OnObservable operators FilterOnObservable, GroupOnObservable, and TransformOnObservable now each have two event tables: one for source changeset events (parent lifecycle) and one for per-item observable events (filter/group/transform emissions). All three explicitly call out that items are invisible downstream until their per-item observable emits at least one value. Per-item observable completion freezes the item in its current state; errors terminate the stream. * docs: link types with see/cref instead of inline code tags Replace <c>IComparer</c>, <c>IEqualityComparer</c>, <c>IEnumerable</c>, <c>Optional<TObject></c>, <c>IChangeSet<TObject, TKey></c>, etc. with proper <see cref='...'/> links for discoverability. * docs: add type links to all param descriptions in cache operators Every param tag now mentions and links its type via see/cref. Source params link to IObservable{T} and IChangeSet{TObject, TKey}, scheduler params link to IScheduler, predicate/factory params link to Func{T, TResult}, comparer params link to IComparer{T} and IEqualityComparer{T}, etc. * docs: add documentation skill codifying XML comment standards Defines the complete process for writing DynamicData operator documentation: analysis steps, XML templates for cache/list/multi-source/mutation/non-changeset operators, quality rules for params/seealso/types/tone, batch application guidance, and concrete examples from Filter, MergeChangeSets, OnItemRemoved, and TransformOnObservable. * docs: add documentation skill for XML comment standards Reusable skill (.github/skills/add-documentation/SKILL.md) codifying the process for writing publication-quality XML documentation for any DynamicData public API: operators, classes, interfaces, enums. Covers analysis, writing templates (cache/list/multi-source/mutation/non-changeset), quality rules (param type linking, bidirectional seealso, tone), batch application, and common mistakes. Replaces the incorrectly placed file. * docs: expand documentation skill with examples and decision criteria Add before/after example (Filter), template selection table, emphasize additive-only rule, and note about Refresh behavior variance across operators. Restructure for clarity. * docs: inline unified template in documentation skill Replace scattered template fragments with a single copy-and-delete template covering all operator categories (cache, list, multi-source, mutation, non-changeset). One file, no duplication, delete the sections that don't apply. * docs: distinguish delegating vs independent overloads in skill Not all secondary overloads delegate to a primary. When an overload has a different implementation or different behavior, it needs its own full documentation, not just inheritdoc. * Revert "docs: add type links to all param descriptions in cache operators" This reverts commit 13241c0. * docs: add type links to cache operator param descriptions Surgical param-by-param replacement: each param's type determined from the actual method signature and linked via see/cref. 344/618 params linked in main file, 7/25 in VirtualiseAndPage. All existing event tables, seealso, inheritdoc, and worth-noting sections preserved intact. * docs: add type links to cache operator param descriptions Surgical param-by-param replacement preserving all existing event tables, seealso, inheritdoc, and worth-noting sections. 528/618 params linked (85%). Remaining 90 are exempt (bool, int, enum, domain-type params). * docs: link remaining non-primitive param types Fix multiline params (pollingInterval, resultGroupSource) and add typeparamref links for domain-type params (key, item). 570/618 linked (92%). Remaining 48 are bool/int primitives. * docs: use see langword for C# keywords (true, false, null) Replace <c>true</c>, <c>false</c>, <c>null</c> with <see langword='true'/>, <see langword='false'/>, <see langword='null'/> for proper keyword rendering in generated HTML documentation. Also update documentation skill with langword guidance. * docs: add cross-file seealso between cache and list operators RemoveKey now links to ObservableListEx.AddKey (the inverse operation that converts list changesets to cache changesets by adding keys). * docs: add cross-links from cache operators to list equivalents 27 primary cache operators now link to their list counterpart via seealso, enabling navigation between the two collection types in generated docs. * Fix param descriptions: proper English with type links, fix Optional.None cref, update skill - Rewrite 470+ param descriptions to read as natural English with type links woven in - Fix double-brace cref syntax ({{T}} to {T}) - Fix wrong types in cref attributes (IObservable{T} to actual parameter types) - Fix meaningless descriptions (the destination, the updater) with specific text - Fix double-description patterns (that The) - Link Optional.None references as <see cref='Optional.None{T}'/> - Fix useReplaceForUpdates, resorter, regrouper, forceTransform params - Update documentation skill with comprehensive param writing guidance * Perfect param descriptions: purpose verbs, combined crefs, no redundancy - Add operator-specific purpose verbs to 150+ source params (to filter, to sort, to join, etc.) - Fix mutation helper params with specific purposes (to add or update items in, to clear, etc.) - Combine all split IObservable{T}/IChangeSet crefs into single nested format - Remove 'changeset stream' redundancy after IChangeSet type references - Remove type name echoes (comparer after IComparer, scheduler after IScheduler) - Fix broken 'tion' fragments, doubled 'items items', grammar issues - Add type links to SortAndBind and VirtualiseAndPage source params - Update documentation skill with comprehensive param writing guidance and examples * Remove OnError/OnCompleted rows from synchronous void mutation methods These rows are meaningless on void methods (AddOrUpdate, Clear, EditDiff) that don't participate in Rx streams. Event tables on these methods now only describe the changeset events they produce (Add, Update, Remove, Refresh). * Fix review findings: dangling prepositions, generic verbs, broken fragments - Replace 'to process.' with operator-specific verbs (6 params) - Rewrite 'to subscribe to per-item X from.' dangling prepositions (20+ params) - Fix 'tion that' broken word fragments in transformFactory params (2) - Fix 'An optional X optional:' double-optional patterns (5+ params) - Rewrite 'a function returning' mangled text in timeSelector params - Add purpose to readOnlyObservableCollection and MergeChangeSets other params * Fix SortAndBind and VirtualiseAndPage params: add type links and purposes - Add type links to all readOnlyObservableCollection params (ReadOnlyObservableCollection{TObject}) - Add type links to targetList params (IList{TObject}) - Add type links to comparer params (IComparer{TObject}) - Add type links to comparerChanged params (IObservable{IComparer{TObject}}) - Add type links to virtualRequests and pageRequests params - Fix source params missing type links ('The source changeset stream.' to proper cref) - Fix 'The size.' useless description - Remove type echoes ('virtualising requests' after IVirtualRequest) * Eliminate trailing prepositions in param descriptions Rewrite 'to remove items from.' to 'from which to remove items.' and similar patterns across Remove, RemoveKey, RemoveKeys methods.
1 parent c1c312b commit 12c1947

File tree

4 files changed

+3310
-2575
lines changed

4 files changed

+3310
-2575
lines changed
Lines changed: 269 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,269 @@
1+
---
2+
name: add-documentation
3+
description: Use when adding or updating XML documentation for any public API in DynamicData, including new operators, changed behavior, batch documentation passes, or PR reviewer requests for improved docs. Covers extension methods, public classes, interfaces, enums, and any member visible in the published API reference.
4+
---
5+
6+
# Add Documentation
7+
8+
## Overview
9+
10+
Write publication-quality XML documentation for DynamicData public APIs. Every behavioral claim must be traced from the implementation source, not guessed from naming conventions. The documentation will be extracted as HTML and published on the ReactiveUI docs site.
11+
12+
## When to Use
13+
14+
- New operator or public API added
15+
- Existing operator's behavior changed
16+
- Batch documentation pass across a file
17+
- PR reviewer requests improved documentation
18+
- Any public type, interface, enum, or member needs docs
19+
20+
## Core Rule: Additive Only
21+
22+
Never remove existing useful information from comments. After making changes, diff against main to confirm nothing substantive was lost.
23+
24+
## Unified Template
25+
26+
Start from this template for any primary overload. Delete sections that do not apply.
27+
28+
```xml
29+
/// <summary>
30+
/// [1-3 sentences: what it does, why you'd use it. No behavioral details.]
31+
/// </summary>
32+
/// <typeparam name="TObject">The type of items.</typeparam>
33+
/// <typeparam name="TKey">The type of the key.</typeparam>
34+
/// <param name="source">The source <see cref="IObservable{IChangeSet{TObject, TKey}}"/> to [verb: filter, transform, sort, etc.].</param>
35+
/// <param name="exampleParam">An <see cref="IComparer{TObject}"/> that determines sort order.</param>
36+
/// <returns>[What it emits and what each emission represents.]</returns>
37+
/// <exception cref="ArgumentNullException">Thrown when <paramref name="source"/> is <c>null</c>.</exception>
38+
/// <remarks>
39+
/// <para>[When to use. How it differs from alternatives. Multiple paragraphs fine.]</para>
40+
///
41+
/// <!-- SINGLE-SOURCE CHANGESET OPERATOR: one event table -->
42+
/// <!-- MULTI-SOURCE OPERATOR: separate labeled tables per source (see below) -->
43+
/// <!-- MUTATION HELPER: table describes what changeset is PRODUCED -->
44+
/// <!-- NON-CHANGESET OUTPUT: table describes how source events affect subscriptions -->
45+
///
46+
/// <list type="table">
47+
/// <listheader><term>Event</term><description>Behavior</description></listheader>
48+
/// <!-- Cache operators -->
49+
/// <item><term>Add</term><description>[specific outcome, bold output reasons]</description></item>
50+
/// <item><term>Update</term><description>[specific outcome]</description></item>
51+
/// <item><term>Remove</term><description>[specific outcome, mention cleanup]</description></item>
52+
/// <item><term>Refresh</term><description>[specific outcome, trace carefully]</description></item>
53+
/// <!-- List operators: also include these -->
54+
/// <item><term>AddRange</term><description>[specific outcome]</description></item>
55+
/// <item><term>Replace</term><description>[list equivalent of Update]</description></item>
56+
/// <item><term>RemoveRange</term><description>[specific outcome]</description></item>
57+
/// <item><term>Moved</term><description>[specific outcome]</description></item>
58+
/// <item><term>Clear</term><description>[specific outcome]</description></item>
59+
/// <!-- All operators -->
60+
/// <item><term>OnError</term><description>[forwarded? swallowed? conditional?]</description></item>
61+
/// <item><term>OnCompleted</term><description>[immediate? waits for children?]</description></item>
62+
/// </list>
63+
///
64+
/// <!-- MULTI-SOURCE: use this pattern instead of the single table above -->
65+
/// <para><b>Source changeset handling (parent events):</b></para>
66+
/// <list type="table">
67+
/// <listheader><term>Event</term><description>Behavior</description></listheader>
68+
/// <item><term>Add</term><description>[subscribes to child / creates state]</description></item>
69+
/// <item><term>Update</term><description>[disposes old, subscribes to new]</description></item>
70+
/// <item><term>Remove</term><description>[disposes, emits downstream removes]</description></item>
71+
/// <item><term>Refresh</term><description>[no effect / re-evaluates]</description></item>
72+
/// </list>
73+
/// <para><b>Per-item observable handling:</b></para>
74+
/// <list type="table">
75+
/// <listheader><term>Emission</term><description>Behavior</description></listheader>
76+
/// <item><term>First value</term><description>[what appears downstream]</description></item>
77+
/// <item><term>Subsequent values</term><description>[updates? replacements?]</description></item>
78+
/// <item><term>Error</term><description>[terminates? swallowed?]</description></item>
79+
/// <item><term>Completed</term><description>[freezes? removed?]</description></item>
80+
/// </list>
81+
///
82+
/// <para><b>Worth noting:</b> [Non-obvious behavior, edge cases, disposal semantics.]</para>
83+
/// </remarks>
84+
/// <seealso cref="[OtherOverloadsInSet]"/>
85+
/// <seealso cref="[SafeOrAsyncVariant]"/>
86+
/// <seealso cref="[SimilarOperator]"/>
87+
/// <seealso cref="[ComplementaryOperator]"/>
88+
```
89+
90+
For **secondary overloads** in an overload set:
91+
```xml
92+
<!-- When the overload delegates to the primary (simpler signature, fewer params): -->
93+
/// <inheritdoc cref="[PrimarySignature]"/>
94+
/// <param name="[delta]">A <see cref="[Type]"/> that [specific difference].</param>
95+
/// <remarks>This overload [what differs]. Delegates to <see cref="[Primary]"/>.</remarks>
96+
97+
<!-- When the overload has a different implementation (different input type, different behavior): -->
98+
<!-- Use the full unified template above. It needs its own complete documentation. -->
99+
<!-- Still link to the other overloads via seealso for discoverability. -->
100+
```
101+
102+
## Process
103+
104+
### 1. Analyze the Implementation
105+
106+
Read the actual source code. Trace each code path for every change reason the operator handles.
107+
108+
Key questions:
109+
- What does each change reason produce downstream? Name the exact output.
110+
- Are there conditional outcomes? (e.g., Filter's Update has four possible results)
111+
- Does the operator create per-item subscriptions? When created/disposed?
112+
- Are errors from child subscriptions forwarded or swallowed?
113+
- Does OnCompleted wait for child subscriptions?
114+
115+
Refresh behavior varies significantly between operators: some re-evaluate (Filter), some forward as-is (Transform by default), some drop (FilterImmutable), some convert to other change types. But all change reasons deserve the same careful tracing.
116+
117+
### 2. Choose What to Include
118+
119+
| The method... | Template sections to use |
120+
|---|---|
121+
| Extends `IObservable<IChangeSet>`, produces `IObservable<IChangeSet>` | Single event table (cache or list rows) |
122+
| Has multiple input sources or per-item observables | Multi-source tables (parent + child) |
123+
| Extends `ISourceCache`/`ISourceList`, returns void | Single table, framed as "produced" |
124+
| Produces non-changeset output (`IObservable<T>`, `bool`, etc.) | Single table, framed as subscription lifecycle |
125+
| Is a class, interface, enum, or non-extension member | Summary, params, returns, remarks (no event table) |
126+
127+
### 3. Apply Quality Rules
128+
129+
**Params**: Every `<param>` must read as natural English with the type linked via `<see cref="..."/>` woven into the sentence. No type is exempt from linking (including enums, `Optional`, `Change`, `IChangeSet`, standard library types like `IComparer`, `TimeSpan`, `IScheduler`). Use `<see langword="true"/>` / `<see langword="false"/>` / `<see langword="null"/>` for C# keywords.
130+
131+
Param writing rules:
132+
- **Reads as English.** Read it aloud. If it sounds wrong, it IS wrong.
133+
- **Starts with an article** ("The", "A", "An") or a condition ("When", "If").
134+
- **Links the actual parameter type** from the method signature. Use `<see cref="..."/>`. Get the generic type arguments right (e.g., `IComparer{TObject}` not `IComparer{T}` when the param type is `IComparer<TObject>`).
135+
- **Describes the purpose**, not just the type. Every param description must answer: "what does the caller use this for?"
136+
- **No redundancy with the type name.** The type `IChangeSet` already says "changeset", so don't add "changeset stream" after it. The type `IObservable` already says "observable", so don't add "observable" after it. The type `IComparer` already says "comparer", so don't add "comparer" after it.
137+
- **No double articles.** "The source [type] the left stream" has two articles fighting each other.
138+
- **Combined nested crefs** for observable changeset types. Write `IObservable{IChangeSet{TObject, TKey}}` as one cref. NEVER split into `IObservable{T}` + "of" + `IChangeSet{...}`.
139+
- **For deeply nested generics** (3+ nesting levels), use `{T}` in the cref and describe the actual type in prose.
140+
- **For `params` array parameters**, do not include `[]` in the cref.
141+
- **For `Optional.None`**, use `<see cref="Optional.None{T}"/>`.
142+
143+
```xml
144+
<!-- ======================== BAD EXAMPLES ======================== -->
145+
146+
<!-- BAD: split type into two parts -->
147+
/// <param name="source">The source <see cref="IObservable{T}"/> of <see cref="IChangeSet{TObject, TKey}"/>.</param>
148+
149+
<!-- BAD: redundant "changeset stream" after IChangeSet type -->
150+
/// <param name="source">The source <see cref="IObservable{IChangeSet{TObject, TKey}}"/> changeset stream.</param>
151+
152+
<!-- BAD: no purpose, just the type name -->
153+
/// <param name="comparer">The <see cref="IComparer{TObject}"/>.</param>
154+
155+
<!-- BAD: echoes the type name ("comparer" after IComparer) -->
156+
/// <param name="comparer">The <see cref="IComparer{TObject}"/> comparer used for sorting.</param>
157+
158+
<!-- BAD: double article ("The source ... the left") -->
159+
/// <param name="left">The source <see cref="IObservable{IChangeSet{TLeft, TLeftKey}}"/> the left input.</param>
160+
161+
<!-- ======================== GOOD EXAMPLES ======================== -->
162+
163+
<!-- source params: "The source [type] to [operator verb]." -->
164+
/// <param name="source">The source <see cref="IObservable{IChangeSet{TObject, TKey}}"/> to filter.</param>
165+
/// <param name="source">The source <see cref="IObservable{IChangeSet{TObject, TKey}}"/> to transform.</param>
166+
/// <param name="source">The source <see cref="IObservable{ISortedChangeSet{TObject, TKey}}"/> to bind.</param>
167+
/// <param name="source">The <see cref="ISourceCache{TObject, TKey}"/> to add items to.</param>
168+
169+
<!-- join left/right params -->
170+
/// <param name="left">The left <see cref="IObservable{IChangeSet{TLeft, TLeftKey}}"/> to join.</param>
171+
/// <param name="right">The right <see cref="IObservable{IChangeSet{TRight, TRightKey}}"/> to join.</param>
172+
173+
<!-- destination/output params: describe what it receives -->
174+
/// <param name="destination">The <see cref="IObservableCollection{TObject}"/> that will receive the changes.</param>
175+
/// <param name="readOnlyObservableCollection">The output <see cref="ReadOnlyObservableCollection{TObject}"/> that will be populated with the results.</param>
176+
177+
<!-- comparer/equality params: describe what they control -->
178+
/// <param name="comparer">The <see cref="IComparer{TObject}"/> that determines sort order.</param>
179+
/// <param name="equalityComparer">An optional <see cref="IEqualityComparer{TObject}"/> for determining item equality.</param>
180+
181+
<!-- factory/selector params: describe what the function does -->
182+
/// <param name="transformFactory">A <see cref="Func{TSource, TDestination}"/> that projects each source item into a destination item.</param>
183+
/// <param name="filter">A <see cref="Func{TObject, bool}"/> predicate that determines which items to include.</param>
184+
/// <param name="rightKeySelector">A <see cref="Func{TRight, TLeftKey}"/> that extracts the join key from each right item.</param>
185+
186+
<!-- scheduler/options params: describe what they configure -->
187+
/// <param name="scheduler">An optional <see cref="IScheduler"/> for scheduling expiry timers.</param>
188+
/// <param name="options">The <see cref="BindingOptions"/> that controls reset threshold and binding behavior.</param>
189+
190+
<!-- bool params: describe the effect -->
191+
/// <param name="transformOnRefresh">When <see langword="true"/>, re-invokes the transform factory on Refresh changes instead of forwarding them.</param>
192+
/// <param name="invokeOnUnsubscribe">When <see langword="true"/>, invokes the callback for all tracked items when the subscription is disposed.</param>
193+
```
194+
195+
**SeeAlso**: Bidirectional for overload sets. Link safe/async/immutable variants, similar operators, complementary operators, commonly confused operators.
196+
197+
**Type references**: All types use `<see cref="..."/>`, including enums, structs, and standard library types. Method/event/property names use `<c>...</c>`. Internal types must not appear; describe behavior instead.
198+
199+
**Tone**: No em dashes. No emoji. No filler words (comprehensive, robust, seamlessly, leverage, utilize, facilitate). Be specific: "an **Update** is emitted" not "the change is propagated". Use "Worth noting" for non-obvious behavior.
200+
201+
### 4. Verify
202+
203+
```bash
204+
dotnet build src/DynamicData/DynamicData.csproj --no-restore -c Release --framework net9.0
205+
```
206+
- 0 errors, no new CS1574 warnings
207+
- No em dashes (Unicode 0x2014)
208+
- No "The source.</param>" remaining
209+
- Spot-check 3-5 operators against implementation
210+
- Diff against main: confirm nothing lost
211+
212+
## Before and After
213+
214+
**BEFORE** (old-style):
215+
```xml
216+
/// <summary>
217+
/// Filters the specified source.
218+
/// </summary>
219+
/// <param name="source">The source.</param>
220+
/// <param name="filter">The filter.</param>
221+
/// <returns>An observable which emits change sets.</returns>
222+
```
223+
224+
**AFTER** (publication-quality):
225+
```xml
226+
/// <summary>
227+
/// Filters items from the source changeset stream using a static predicate.
228+
/// Only items satisfying <paramref name="filter"/> are included downstream.
229+
/// </summary>
230+
/// <param name="source">The source <see cref="IObservable{T}"/> of <see cref="IChangeSet{TObject, TKey}"/>.</param>
231+
/// <param name="filter">A <see cref="Func{T, TResult}"/> predicate. Items returning <c>true</c> are included.</param>
232+
/// <returns>A changeset stream containing only items satisfying <paramref name="filter"/>.</returns>
233+
/// <remarks>
234+
/// <para>Use this overload when the predicate is fixed for the subscription's lifetime.</para>
235+
/// <list type="table">
236+
/// <listheader><term>Event</term><description>Behavior</description></listheader>
237+
/// <item><term>Add</term><description>Predicate evaluated. If passes, <b>Add</b> emitted. Otherwise dropped.</description></item>
238+
/// <item><term>Update</term><description>Re-evaluated. Both pass: <b>Update</b>. New passes only: <b>Add</b>. Old passed only: <b>Remove</b>. Neither: dropped.</description></item>
239+
/// <item><term>Remove</term><description>If downstream, <b>Remove</b> emitted. Otherwise dropped.</description></item>
240+
/// <item><term>Refresh</term><description>Re-evaluated. Now passes: <b>Add</b>. Still passes: <b>Refresh</b>. No longer passes: <b>Remove</b>. Still fails: dropped.</description></item>
241+
/// <item><term>OnError</term><description>Forwarded.</description></item>
242+
/// <item><term>OnCompleted</term><description>Forwarded.</description></item>
243+
/// </list>
244+
/// <para><b>Worth noting:</b> Refresh re-evaluation can promote or demote items.</para>
245+
/// </remarks>
246+
/// <seealso cref="FilterImmutable{TObject, TKey}"/>
247+
/// <seealso cref="FilterOnObservable{TObject, TKey}"/>
248+
/// <seealso cref="AutoRefresh{TObject, TKey}"/>
249+
```
250+
251+
## Batch Application
252+
253+
1. Dispatch parallel **read-only** agents to analyze implementations (group by family)
254+
2. Apply edits via **single agent or yourself** (multiple agents editing one file clobber each other)
255+
3. Build, audit metrics, spot-check accuracy
256+
4. Diff against main for lost details
257+
258+
## Common Mistakes
259+
260+
| Mistake | Fix |
261+
|---------|-----|
262+
| Guessing behavior from method name | Read the implementation |
263+
| `<c>IComparer</c>` for a type | `<see cref="IComparer{T}"/>` |
264+
| Referencing internal types | Describe behavior instead |
265+
| One-way seealso links | Bidirectional |
266+
| Multiple agents editing same file | One editor at a time |
267+
| Removing existing information | Additive only; diff against main |
268+
| Only cache rows for list operators | List needs AddRange, RemoveRange, Moved, Clear |
269+
| Params missing type links | Every param links its type |

0 commit comments

Comments
 (0)