Overhaul XML documentation for ObservableCacheEx extension methods#1080
Open
dwcullop wants to merge 23 commits intoreactivemarbles:mainfrom
Open
Overhaul XML documentation for ObservableCacheEx extension methods#1080dwcullop wants to merge 23 commits intoreactivemarbles:mainfrom
dwcullop wants to merge 23 commits intoreactivemarbles:mainfrom
Conversation
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
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).
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.
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.
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.
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.
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.
Add before/after example (Filter), template selection table, emphasize additive-only rule, and note about Refresh behavior variance across operators. Restructure for clarity.
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.
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.
…tors" This reverts commit 13241c0.
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.
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).
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.
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.
RemoveKey now links to ObservableListEx.AddKey (the inverse operation that converts list changesets to cache changesets by adding keys).
27 primary cache operators now link to their list counterpart via seealso, enabling navigation between the two collection types in generated docs.
…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
- 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
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).
…gments - 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
…oses
- 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)
Rewrite 'to remove items from.' to 'from which to remove items.' and similar patterns across Remove, RemoveKey, RemoveKeys methods.
RolandPheasant
approved these changes
Apr 15, 2026
Collaborator
RolandPheasant
left a comment
RolandPheasant
left a comment
There was a problem hiding this comment.
Can't properly review as there are so many comments. However the ones I looked at make much sense and are a considerable improvement.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Complete rewrite of XML doc comments across all three
ObservableCacheExpartial class files, covering ~110 unique operators and ~306 total overloads. Every operator now has publication-quality documentation suitable for extraction to the ReactiveUI docs site.Why
The existing XML comments were shallow and generic ("Filters the specified source", "The source", "An observable which emits change sets"). Developers had to read the implementation source to understand how operators actually behave, especially around Refresh handling, error propagation, and completion semantics. This is a reactive collections library where the behavioral details matter: a Refresh event flowing through Filter can become an Add, Remove, forwarded Refresh, or get dropped entirely, and that's not something you can guess from a one-line summary.
What changed
Event behavior tables: 77 operators now have a structured
<list type="table">documenting how each event (Add, Update, Remove, Refresh, OnError, OnCompleted) is handled. Multi-source operators (joins, MergeManyChangeSets, FilterOnObservable, GroupOnObservable, TransformOnObservable) have separate tables per source, so developers can see the parent-side and child-side/per-item-observable behavior independently.Cross-references: 129
<seealso>tags linking related operators: safe variants (Transform/TransformSafe), async variants (Transform/TransformAsync), similar operators (Filter/FilterImmutable/FilterOnObservable), complementary operators (Filter+AutoRefresh, Sort+Bind), and commonly confused operators (MergeMany vs MergeManyChangeSets vs MergeChangeSets).Secondary overloads: 107
<inheritdoc>tags with delta<remarks>for overloads that delegate to a primary, so the behavioral docs aren't duplicated but every overload is still independently browsable."Worth noting" sections: 36 operators have a labeled section calling out non-obvious behavior: OnItemRemoved firing for all tracked items on disposal, MergeChangeSets emitting an Update (not Add) when falling back to another source's value, BatchIf losing buffered data if the source errors while paused, per-item observable operators requiring at least one emission before items become visible, etc.
Summaries trimmed: Every summary is now 1-3 sentences describing what the operator does. All behavioral detail lives in
<remarks>.What didn't change
No code changes. No behavioral changes. No new APIs. All existing useful information in comments was preserved (additive only). Tests: 2,211 passed, 0 failed.