-
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
[kbn/rule-data-utils] add submodules and require public use them #117963
[kbn/rule-data-utils] add submodules and require public use them #117963
Conversation
a709cf8
to
e35993c
Compare
Pinging @elastic/kibana-operations (Team: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.
APM changes look good.
Pinging @elastic/apm-ui (Team:apm) |
Pinging @elastic/uptime (Team:uptime) |
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.
Uptime changes LGTM
@elasticmachine merge upstream |
…ule-data-utils-submodules
@elasticmachine merge upstream |
@@ -850,6 +850,10 @@ module.exports = { | |||
name: 'semver', | |||
message: 'Please use "semver/*/{function}" instead', | |||
}, | |||
{ | |||
name: '@kbn/rule-data-utils', | |||
message: `Import directly from @kbn/rule-data-utils/* submodules in public/common code`, |
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 message specific to rule-data-utils
?
Should this read generically like:
Import directly from @kbn/<module_name>/* in public/common code
or
Import directly from @kbn/*/* in public/common code
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.
Yes, this is specific to the @kbn/rule-data-utils
package as import targets have to be setup for each package individually
{ | ||
"main": "../target_node/technical_field_names", | ||
"types": "../target_types/technical_field_names" | ||
} |
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.
nit: surprised your IDE/prettier isn't adding new lines here.
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.
That's what's defined in the .editorconfig
file, which is automatically managed by the EditorConfig plugin in IDEs https://github.com/elastic/kibana/blob/main/.editorconfig#L12-L13
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.
Read through the ticket, added two small minor items. This is mergeable anytime.
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.
observability
changes look good
…ule-data-utils-submodules
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
…stic#117963) * [kbn/rule-data-utils] add submodules and require public use them * fix lint errors Co-authored-by: Kibana Machine <[email protected]>
💔 Backport failed
Successful backport PRs will be merged automatically after passing CI. To backport manually run: |
…7963) (#118628) * [kbn/rule-data-utils] add submodules and require public use them * fix lint errors Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Spencer <[email protected]>
…igrate-away-from-injected-css-js * 'main' of github.com:elastic/kibana: (221 commits) [Reporting] Add log level to config (elastic#118319) [Metrics UI] Skip failing waffle chart color palette test (elastic#118527) [APM] chore: Unify naming of 'apm/scripts/**/*' with snake_case (elastic#118328) skip flaky suites (elastic#99581, elastic#118356, elastic#118474) [Alerting] Initial implementation of alerting task `cancel()` (elastic#114289) chore(NA): creates pkg_npm_types bazel rule (elastic#116465) skip flaky suite (elastic#116892) Bump chromedriver to 95.0.0 (elastic#116724) [Data visualizer] Improve design of expanded rows (elastic#118125) [APM] prefer ECS field names for HTTP and URL (elastic#118485) Update query_debugging_in_development_and_production.md (elastic#118491) [Uptime] adjust Elastic Synthetics integration functional tests (elastic#118163) [kbn/rule-data-utils] add submodules and require public use them (elastic#117963) [DOCS] Refresh APM correlation screenshots (elastic#116723) (elastic#118577) Handles ns to ms conversion for event loop delay metrics (elastic#118447) [Cases] Rename functional tests folder (elastic#118490) dummy commit skip flaky suite (elastic#118593) Improve workpad schema validation (elastic#97838) skip flaky suite (elastic#118584) ... # Conflicts: # src/plugins/dashboard/public/application/embeddable/viewport/dashboard_viewport.tsx
…stic#117963) * [kbn/rule-data-utils] add submodules and require public use them * fix lint errors Co-authored-by: Kibana Machine <[email protected]> # Conflicts: # x-pack/plugins/apm/server/lib/alerts/register_transaction_duration_anomaly_alert_type.ts # x-pack/plugins/cases/public/components/user_action_tree/index.tsx # x-pack/plugins/observability/public/pages/alerts/alerts_flyout/index.tsx # x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx # x-pack/plugins/security_solution/public/helpers.ts # x-pack/test/case_api_integration/security_and_spaces/tests/common/cases/patch_cases.ts # x-pack/test/case_api_integration/security_and_spaces/tests/common/comments/post_comment.ts
…7963) (#118725) * [kbn/rule-data-utils] add submodules and require public use them * fix lint errors Co-authored-by: Kibana Machine <[email protected]> # Conflicts: # x-pack/plugins/apm/server/lib/alerts/register_transaction_duration_anomaly_alert_type.ts # x-pack/plugins/cases/public/components/user_action_tree/index.tsx # x-pack/plugins/observability/public/pages/alerts/alerts_flyout/index.tsx # x-pack/plugins/security_solution/public/detections/components/alerts_table/actions.tsx # x-pack/plugins/security_solution/public/helpers.ts # x-pack/test/case_api_integration/security_and_spaces/tests/common/cases/patch_cases.ts # x-pack/test/case_api_integration/security_and_spaces/tests/common/comments/post_comment.ts
…stic#117963) * [kbn/rule-data-utils] add submodules and require public use them * fix lint errors Co-authored-by: Kibana Machine <[email protected]>
…7963) * [kbn/rule-data-utils] add submodules and require public use them * fix lint errors Co-authored-by: Kibana Machine <[email protected]>
Similar to #117958, this adds import targets to
@kbn/rule-data-utils
so that we can import sub-modules from the package directly without resorting to wacky hacks. Adds the following import targets and imports from there where appropriate:@kbn/rule-data-utils/alerts_as_data_rbac
@kbn/rule-data-utils/alerts_as_data_severity
@kbn/rule-data-utils/alerts_as_data_status
@kbn/rule-data-utils/technical_field_names
Additionally we've prevented
public
andcommon
code from importing from@kbn/rule-data-utils
directly, to prevent the whole package from ending up in a bundle and bloating it.