-
Notifications
You must be signed in to change notification settings - Fork 8.5k
[ui/public/utils] Copy rarely used items to where they are consumed #53819
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
Conversation
💚 Build SucceededTo update your PR or re-run it, just comment with: |
# Conflicts: # src/legacy/ui/public/vis/editors/config/editor_config_providers.ts
|
Pinging @elastic/kibana-app-arch (Team:AppArch) |
| */ | ||
| export function move( | ||
| objs: object[], | ||
| objs: any[], |
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 did you broaden the type 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.
| import _ from 'lodash'; | ||
| import Bluebird from 'bluebird'; | ||
| import { keyMap } from 'ui/utils/key_map'; | ||
| import { keyMap } from 'ui/directives/key_map'; |
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.
From what I'm seeing, key_map is not a directive.
So why move it there?
lizozom
left a comment
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 LGTM
Added a couple of minor comments.
|
@elasticmachine merge upstream |
pgayvallet
left a comment
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 for platform changes
streamich
left a comment
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.
Code changes LGTM.
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
…lastic#53819) * [ui/public/utils] Copy rarely used items to where they are consumed Closes: elastic#52841 * sort_prefix_first 👉x-pack/legacy/plugins/kuery_autocomplete * numeric 👉src/legacy/core_plugins/kibana/public/management * diff_object + tests 👉ui/state_management * function + tests 👉ui/state_management (function.js was removed!) * key_map 👉ui/directives * leastCommonMultiple 👉ui/vis * string_utils 👉ui/saved_objects * collection * parse_interval * it -> test * fix CI * fix PR comments Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
* master: (55 commits) [ui/public/utils] Copy rarely used items to where they are consumed (elastic#53819) set AppArch team as an owner of the search endpoints (elastic#54131) Don't expose Elasticsearch client as Observable (elastic#53824) [SIEM] Cleanup unnecessary use of enzyme-to-json (elastic#53980) fix ui exports doc (elastic#54138) change markdown element title (elastic#54194) [Logs UI] Refactor log position to hooks (elastic#53540) [SIEM] Implement NP Plugin Setup (elastic#54030) [DOCS] Updates ML links (elastic#53613) sort renovate packages in config Spaces - fix flakey api tests (elastic#54154) Remove dependency that was causing effect to re-execute infinitely. (elastic#54160) [dev/run] expose unexpected flags as more than just names (elastic#54080) [DOCS] Moves index pattern doc to Discover (elastic#53347) [SIEM] Cleanup React imports (elastic#53981) Update eslint related packages (elastic#54107) [Uptime] Added date range filter into expanded list query (elastic#52609) [SIEM] Add react/display-name eslint rule (elastic#53107) [SIEM] Enable eslint prefer-template rule (elastic#53983) Elasticsearch snapshots automation (elastic#53706) ...
…53819) (#54233) * [ui/public/utils] Copy rarely used items to where they are consumed Closes: #52841 * sort_prefix_first 👉x-pack/legacy/plugins/kuery_autocomplete * numeric 👉src/legacy/core_plugins/kibana/public/management * diff_object + tests 👉ui/state_management * function + tests 👉ui/state_management (function.js was removed!) * key_map 👉ui/directives * leastCommonMultiple 👉ui/vis * string_utils 👉ui/saved_objects * collection * parse_interval * it -> test * fix CI * fix PR comments Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
….com:mbondyra/kibana into IS-50036_show-sum-for-a-field-with-formatter * 'IS-50036_show-sum-for-a-field-with-formatter' of github.com:mbondyra/kibana: [Telemetry] Fix license page crashing on telemetry.enabled: fa… (elastic#54174) [ui/public/utils] Copy rarely used items to where they are consumed (elastic#53819) set AppArch team as an owner of the search endpoints (elastic#54131) Don't expose Elasticsearch client as Observable (elastic#53824) [SIEM] Cleanup unnecessary use of enzyme-to-json (elastic#53980)

Closes: #52841
Summary
There are a few utils in
src/legacy/ui/public/utilsthat are small and not widely used. Rather than find a "shared" place for them to live, it is probably cleaner to just copy them to the places that rely on them:case_conversionkeysToSnakeCaseShallow+ testssrc/legacy/server/status/libsrc/legacy/utils/case_conversion.tskeysToCamelCaseShallow+ testssrc/legacy/core_plugins/kibana/public/managementsrc/legacy/utils/case_conversion.tscollectionorganizeBy+ tests 👉src/legacy/ui/public/indexed_arraypushAll+ tests -- delete, unuseddiff_object+ tests 👉ui/state_managementfunctionwas removedkey_map👉ui/directivessrc/test_utils/publicmathleastCommonMultiple👉ui/vis(update imports inui/vis/libandui/vis/editorgreatestCommonDivisor-- move withleastCommonMultiple, but do not export as it is unused outside of this filenumeric👉src/legacy/core_plugins/kibana/public/managementparse_interval👉src/legacy/core_plugins/data/commonsort_prefix_first👉x-pack/legacy/plugins/kuery_autocompletestring_utilssupports👉src/legacy/core_plugins/tile_map/publicChecklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.For maintainers