-
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
SavedObjectsRepository code cleanup (bis) #157363
Conversation
💚 Build Succeeded
Metrics [docs]Unknown metric groupsESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
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.
self-review
// BEWARE: The SavedObjectClient depends on the implementation details of the SavedObjectsRepository | ||
// so any breaking changes to this repository are considered breaking changes to the SavedObjectsClient. |
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: removed those comments because they didn't really bring any value (and the original comment from the repository file was removed in the previous PR)
return [...acc, ...sourceFields.map((f) => `${t}.${f}`)]; | ||
acc.push(...sourceFields.map((f) => `${t}.${f}`)); | ||
return acc; | ||
}, []) | ||
.concat(ROOT_FIELDS) | ||
.concat(fields); // v5 compatibility |
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.
So this is the only behavioral change from the PR: I removed the 'v5' compatibility from the time the type fields were at the top level. (this should have been removed years ago...)
@@ -3792,7 +3792,6 @@ describe('SavedObjectsRepository', () => { | |||
'updated_at', | |||
'created_at', | |||
'originId', | |||
'title', |
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.
See previous comment (on the v5 field compat) for the reason of this test change
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.
self-review
Pinging @elastic/kibana-core (Team:Core) |
## Summary Follow-up of #157154 Continue the cleanup started in the previous PR, by moving more things around. - Move everything search-related (dsl, aggregations, kql utils) to a dedicated `search` folder - Move a few more things to `/apis/internals` - Remove the 'v5' field compatibility in the field list generation (see comment) - Cleanup some files a bit. --------- Co-authored-by: kibanamachine <[email protected]>
Summary
Follow-up of #157154
Continue the cleanup started in the previous PR, by moving more things around.
search
folder/apis/internals