-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Search platform context cleanup #57448
Search platform context cleanup #57448
Conversation
Pinging @elastic/kibana-app-arch (Team:AppArch) |
@elasticmachine merge upstream |
…om/kibana into newplatform/search/context-cleanup
…earch/context-cleanup
@elasticmachine merge upstream |
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.
Overall this is looking really good, thanks for all of the cleanup! Other than what's listed below, I wanted to ask if it makes sense to move the types in the server/
folder into a dedicated types file like you've done with public/
, or if you'd rather wait until we're modifying that code to do it.
*/ | ||
|
||
import { CoreStart } from 'kibana/public'; | ||
import { ISearch, ISearchGeneric } from './i_search'; |
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 there any reason you didn't move ISearch
and ISearchGeneric
into this file?
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.
Because they are used in examples/search_explorer/public/search_api.tsx
and I thought they might be important enough to stay there.
…earch/context-cleanup
…om/kibana into newplatform/search/context-cleanup
Remove empty file reference
…earch/context-cleanup
…om/kibana into newplatform/search/context-cleanup
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.
Pulled & tested locally and things seem to be working! Thanks!
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
* Initial commit - cleanup * cleanup * tsing * ts fixes * Fix jest test * Code review fxes * Remove empty file reference Remove empty file reference * code review Co-authored-by: Elastic Machine <[email protected]>
* Initial commit - cleanup * cleanup * tsing * ts fixes * Fix jest test * Code review fxes * Remove empty file reference Remove empty file reference * code review Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Elastic Machine <[email protected]>
Summary
Resolves #54074
Removes the usage of core platform context and replaces it with
getDependencies()
along side some cleanup to bring the search service to a similar state as the other services in data plugin.Checklist
Delete any items that are not applicable to this PR.
For maintainers