-
Notifications
You must be signed in to change notification settings - Fork 8.5k
[dashboards as code] remove content management #243093
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
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
/ci |
|
@elasticmachine merge upstream |
|
Pinging @elastic/kibana-presentation (Team:Presentation) |
💚 Build Succeeded
Metrics [docs]Module Count
Public APIs missing comments
Async chunks
Page load bundle
History
|
kowalczyk-krzysztof
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.
SharedUX code changes LGTM
nickpeihl
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! just one question about constants.
code review and tested updated endpoints.
| export const LANDING_PAGE_PATH = '/list'; | ||
| export const DASHBOARD_APP_ID = 'dashboards'; | ||
| export const SEARCH_SESSION_ID = 'searchSessionId'; | ||
| export const DASHBOARD_GRID_COLUMN_COUNT = 48; |
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.
I'm not sure I'm fully understanding the constants separation here. I'm guessing these constants are expected to be in the public bundle as opposed to server-only. But then shouldn't the DEFAULT_PANEL_WIDTH, DEFAULT_PANEL_HEIGHT, and DEFAULT_DASHBOARD_OPTIONS constants be here since they are also used in the public bundle?
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.
one file is included in plugin page load. The other file, is async loaded when dashboard application is loaded. The split avoids putting constants into page load that are not needed.
| */ | ||
|
|
||
| export * from './get'; | ||
| export * from './common'; |
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.
explicit exports! 💯
Closes #238500
Closes #241368
Closes #240161
PR decouples GET endpoint from content management. PR then removes content management since routes are no longer using it.