-
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
[SecuritySolution] Fix TypeScript errors #167147
Conversation
@@ -5,6 +5,8 @@ | |||
* 2.0. | |||
*/ | |||
|
|||
// TODO: This import can be removed, once https://github.com/elastic/eui/pull/7221 made its way into Kibana main | |||
import type { DefaultEuiMarkdownProcessingPlugins } from '@elastic/eui/src/components/markdown_editor/plugins/markdown_default_plugins/processing_plugins'; |
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.
Linking elastic/eui#7221 here to keep track of it
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.
@janmonschke , now that the linked PR is merged. Do you want to go ahead and remove the import in this PR only?
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.
@logeekal Oh, did they already update EUI in Kibana?
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.
nope, #166868
7fdabe8
to
778df3a
Compare
}; | ||
export const nonStatefulUiPlugins = getDefaultEuiMarkdownUiPlugins(); | ||
export const parsingPlugins = getDefaultEuiMarkdownParsingPlugins(); | ||
// TODO: This explicit type can be removed, once https://github.com/elastic/eui/pull/7221 made its way into Kibana main |
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.
Linking elastic/eui#7221 here to keep track of it
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.
Thanks!
@elasticmachine merge upstream |
1cb66cf
to
16ef116
Compare
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.
LGTM from explore side, thanks for the fix!
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.
Thanks for making the fixes!
16ef116
to
d9c2abf
Compare
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've verified that the 11 TypeScript errors coming from x-pack/test_serverless/tsconfig.json
in the "Check Types Commit Diff" check are not related to this PR and so we can safely merge it as long as the other CI checks are green and all reviews are positive.
…-ref HEAD~1..HEAD --fix'
c6a0d20
to
9e90e93
Compare
💔 Build FailedFailed CI Steps
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
Force merged to get around unrelated TypeScript errors and flaky tests |
Summary
Fixes various TypeScript errors in security solution and associated modules/plugins.