-
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
Move ui/indices into es_ui_shared plugin. #60186
Changes from 4 commits
6518c3c
911640d
0a61eb6
7294373
6bc97b3
4059341
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,4 +6,4 @@ | |
- `filter` | ||
- `index_patterns` | ||
- `query` | ||
- `search` | ||
- `search` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,6 @@ import PropTypes from 'prop-types'; | |
import { debounce } from 'lodash'; | ||
import { i18n } from '@kbn/i18n'; | ||
import { FormattedMessage } from '@kbn/i18n/react'; | ||
import { INDEX_ILLEGAL_CHARACTERS_VISIBLE } from 'ui/indices'; | ||
import { fatalError } from 'ui/notify'; | ||
|
||
import { | ||
|
@@ -30,6 +29,7 @@ import { | |
EuiTitle, | ||
} from '@elastic/eui'; | ||
|
||
import { INDEX_ILLEGAL_CHARACTERS_VISIBLE } from '../../../../../../../../src/plugins/es_ui_shared/public'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be better to namespace our export. import { indices } from '../../../../../../../../src/plugins/es_ui_shared/public;
const { INDEX_ILLEGAL_CHARACTERS_VISIBLE } = indices; This will avoid naming collisions and better organization of all our exports. |
||
import { indexNameValidator, leaderIndexValidator } from '../../services/input_validation'; | ||
import routing from '../../services/routing'; | ||
import { loadIndices } from '../../services/api'; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,8 +12,7 @@ import { | |
indexNameBeginsWithPeriod, | ||
findIllegalCharactersInIndexName, | ||
indexNameContainsSpaces, | ||
} from 'ui/indices'; | ||
|
||
} from '../../../../../../../src/plugins/es_ui_shared/public'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, it would be better to namespace under |
||
import { indexPatterns } from '../../../../../../../src/plugins/data/public'; | ||
|
||
export const validateName = (name = '') => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
export { | ||
findIllegalCharactersInIndexName, | ||
INDEX_ILLEGAL_CHARACTERS_VISIBLE, | ||
} from '../../../../../src/plugins/es_ui_shared/public'; |
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.
@sebelga AFAICT this file isn't consumed anywhere, so I wasn't sure how to test this. I don't want to spend time writing a test for it. I'm inclined to leave it with the expectation that a problem will be surfaced and fixed if/when we consume this file. What are your thoughts?
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.
OK, although I think it would have been nice to make use of this single validator in our apps instead of re-writing it multiple times as I've seen. Maybe as a second pass 😊