-
-
Notifications
You must be signed in to change notification settings - Fork 658
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
chore: split interfaces for import and export #5004
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
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.
One important comment inside
src/lib/types/services.ts
Outdated
exportImportService: ExportImportService; | ||
exportImportServiceV2: WithTransactional<ExportImportService>; | ||
exportService: IExportService; | ||
importService: WithTransactional<ExportImportService>; |
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.
Why not IExportService after you split them?
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.
I believe you mean, why not IImportService, right? Now, export doesn't have to be transactional and we can use the same and we only need import to be transactional
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.
Let me know if 6950fc1 is what you're expecting ;)
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.
correct :)
About the changes
This splits the interfaces for import and export, especially because the import functionality has to be replaced in enterprise repo.
This is a breaking change because of the service renames, but I'll have the PR for the other repository ready so we reduce the time to fix. I intentionally avoided doing it backward compatible because of time.