-
Notifications
You must be signed in to change notification settings - Fork 6
feat: add unified API documentation #368
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat: add unified API documentation #368
Conversation
laurenzlong
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
awesome start!
…-2276-expand-ampyaml-examples-to-include-more-unified-models
…-2276-expand-ampyaml-examples-to-include-more-unified-models
|
|
||
| ```json | ||
| { | ||
| "mappedFields": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@laurenzlong should we use it exactly like merge.dev?
goes back on being confusing when using:
remote_id (provider's internal id) and id (merge's internal id)
vs.
id (ref provider's internal id) and ampersand_id (for example, to reference our internal id - if that's the case)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for this example, I'd match the webhook to your yaml file above (so only include company_name and revenue in mapped fields)
but the Github examples should include remote_id since those are meant to be "merge-compatible"
laurenzlong
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So sorry for my delay in reviewing this! Good to merge once you:
- Address comments
- Run
pnpm gen(it's not automatic)
|
|
||
| **Merge.dev:** | ||
| - **Read**: Scheduled syncs + poll after webhook notifications (model-level) | ||
| - **Write**: Instant POST/PATCH operations |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this true? does merge directly make the write to the provider right away? also do you know if they apply mappings in their writes? if not, then that's worth highlighting.
I'd also remove the word "instant", it sounds too positive.
| | **Event detail** | Field-level changes | Model-level notifications | | ||
| | **Sub-processor** | No | Yes | | ||
|
|
||
| ## Examples |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this section can be removed, since there's a "Complete examples" section below already, and we want people to get to the "Migration from merge.dev" section faster.
|
|
||
| A unified API provides a single, consistent interface to interact with multiple third-party services. Instead of learning each provider's unique schema, you define one schema that works across all of them. | ||
|
|
||
| <Note> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesn't need to be in a "Note", we save those for special call outs.
|
|
||
| - **Faster development** - Add providers without changing application logic | ||
| - **Consistent data model** - One schema regardless of source | ||
| - **Real-time updates** - Subscribe to field-level changes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure this is a strong bullet, it's a bit confusing why this is a reason to build a unified API.
src/unified-api.mdx
Outdated
|
|
||
| **Ampersand:** No data storage. Data streams directly from providers to your webhooks. You control where and how to persist data. | ||
|
|
||
| **Merge.dev:** Stores all customer data in their cloud by default. Enables faster syncs but requires them as a data sub-processor. "Merge Destinations" (Enterprise) bypasses storage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here and everywhere else in the doc, can you change "merge.dev" to "Merge"? Since that's the actual company name. n
Co-authored-by: Lauren Long <laurenzlong@users.noreply.github.com>
Co-authored-by: Lauren Long <laurenzlong@users.noreply.github.com>
Co-authored-by: Lauren Long <laurenzlong@users.noreply.github.com>
Co-authored-by: Lauren Long <laurenzlong@users.noreply.github.com>
Co-authored-by: Lauren Long <laurenzlong@users.noreply.github.com>
Co-authored-by: Lauren Long <laurenzlong@users.noreply.github.com>
Co-authored-by: Lauren Long <laurenzlong@users.noreply.github.com>
Co-authored-by: Lauren Long <laurenzlong@users.noreply.github.com>
Co-authored-by: Lauren Long <laurenzlong@users.noreply.github.com>
Co-authored-by: Lauren Long <laurenzlong@users.noreply.github.com>
|
Hey @caiopizzol it looks like you re-requested a review from me, it looks like there are still some comments that haven't been addressed. You can actually merge this once those comments are addressed, but feel free to re-request review if you wanted me to take another look. |
Description
Add a new, dedicated, unified API documentation page including how to migrate from Merge.dev to Ampersand.
Screenshot