-
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
[ui/public/utils] Copy rarely used items to where they are consumed #53819
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) |
@@ -33,10 +33,10 @@ import _ from 'lodash'; | |||
* @return {array} - the objs argument | |||
*/ | |||
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.
@@ -20,7 +20,7 @@ | |||
import $ from 'jquery'; | |||
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?
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 |
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
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/utils
that 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_conversion
keysToSnakeCaseShallow
+ testssrc/legacy/server/status/lib
src/legacy/utils/case_conversion.ts
keysToCamelCaseShallow
+ testssrc/legacy/core_plugins/kibana/public/management
src/legacy/utils/case_conversion.ts
collection
organizeBy
+ tests 👉src/legacy/ui/public/indexed_array
pushAll
+ tests -- delete, unuseddiff_object
+ tests 👉ui/state_management
function
was removedkey_map
👉ui/directives
src/test_utils/public
math
leastCommonMultiple
👉ui/vis
(update imports inui/vis/lib
andui/vis/editor
greatestCommonDivisor
-- move withleastCommonMultiple
, but do not export as it is unused outside of this filenumeric
👉src/legacy/core_plugins/kibana/public/management
parse_interval
👉src/legacy/core_plugins/data/common
sort_prefix_first
👉x-pack/legacy/plugins/kuery_autocomplete
string_utils
supports
👉src/legacy/core_plugins/tile_map/public
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.For maintainers