Add ADR for Samples catalogue - #110
Conversation
emmanuelmathot
left a comment
emmanuelmathot
left a comment
There was a problem hiding this comment.
Nice ADR, thx @danielfdsilva. I like the pagination-cursor reasoning. A few things worth resolving before merge in the inline comments.
| providerIds?: string[]; // registry-only: restrict fan-out to these providers | ||
| } | ||
|
|
||
| interface SampleSourceProvider { |
There was a problem hiding this comment.
Naming still says "Sample*" despite the catalogue reframe**
docs/adr/0004-catalogue-provider-architecture.md:94
I personally prefer "Sample" wording but it can be the sub granule of a sample catalog.
There was a problem hiding this comment.
This was meant to be an example, but will update to look more final
| - Good: the registry can drop a failing provider without affecting the others. | ||
| - Good: id namespacing (below) means an editor deep-link resolves with exactly one provider call, not a fan-out over every active provider. | ||
| - Bad: the catalogue UI cannot show one combined "page 3 of 47" across all sources. It must treat each provider as an independently-paginated section instead (see below). | ||
| - Bad: existing static-list consumers (`EditorPage`, `samples.tsx`, the landing page) move from synchronous access to hooks with a loading state. |
There was a problem hiding this comment.
Fine to leave the hook design for the implementation PR, but let's make sure it's tracked as a follow-up so it doesn't disappear.
emmanuelmathot
left a comment
emmanuelmathot
left a comment
There was a problem hiding this comment.
Two suggestions to close out the remaining threads.
Co-authored-by: Emmanuel Mathot <emmanuel.mathot@gmail.com>
2 participants
Contributes to #102
@emmanuelmathot Let me know what you think of this approach.