-
Notifications
You must be signed in to change notification settings - Fork 10.7k
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: Move service definitions to core services package #27546
Chore: Move service definitions to core services package #27546
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #27546 +/- ##
===========================================
+ Coverage 42.46% 43.71% +1.25%
===========================================
Files 839 812 -27
Lines 17695 17176 -519
Branches 1977 1902 -75
===========================================
- Hits 7514 7509 -5
+ Misses 9929 9416 -513
+ Partials 252 251 -1
Flags with carried forward coverage won't be shown. Click here to find out more. |
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.
@rocket.chat/core-sdk
its not a good naming strategy since we can have sdk for client and server
maybe @rocket.chat/serve-core-sdk
?
This PR currently has a merge conflict. Please resolve this and then re-add the |
@@ -4,11 +4,11 @@ import type { IMessage } from '@rocket.chat/apps-engine/definition/messages'; | |||
import type { IUser } from '@rocket.chat/apps-engine/definition/users'; | |||
import type { IRoom } from '@rocket.chat/apps-engine/definition/rooms'; | |||
import type { ISubscription } from '@rocket.chat/core-typings'; | |||
import { api } from '@rocket.chat/core-services'; |
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'm voting for renaming api
to core
.
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.
🤔 interesting
…itions-to-core-sdk-package
This PR currently has a merge conflict. Please resolve this and then re-add the |
Proposed changes (including videos or screenshots)
Issue(s)
ARCH-118
Steps to test or reproduce
Further comments
There are a few things missing:
StreamerCentral
toLocalBroker
via dependency injectionapi
export fromapps/meteor/server/sdk/api.ts
getRoutingManagerConfig
was removed, check if it was really not being usedpackages/core-sdk/src/types/IVoipService.ts
check howgetConnector()
is used and figure how to fix it out