Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Don't bootstrap core type emits #66377

Merged
merged 1 commit into from
May 15, 2020
Merged

Don't bootstrap core type emits #66377

merged 1 commit into from
May 15, 2020

Conversation

rudolf
Copy link
Contributor

@rudolf rudolf commented May 13, 2020

Summary

Importing types from kibana/server or kibana/public used to rely on .d.ts files in target/types, so as part of bootstrap we generated these types so that IDE type checking won't fail.

This is no longer necessary since kibana.d.ts now imports types from their source instead of target/types so I've removed this expensive step from the bootstrap process.

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@rudolf rudolf added chore Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc release_note:skip Skip the PR/issue when compiling release notes v7.9.0 labels May 13, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@botelastic botelastic bot added the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label May 13, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@rudolf rudolf marked this pull request as ready for review May 13, 2020 12:45
@rudolf rudolf requested review from a team as code owners May 13, 2020 12:45
FrankHassanabad
FrankHassanabad approved these changes May 13, 2020
View reviewed changes
Copy link
Contributor

@FrankHassanabad FrankHassanabad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SIEM side looks good, thank you for keeping all these solutions imports clean and thank you for all the work to improve the quality of everyone's code over time! Couldn't be happier with all gifts like the improved Types and changes to imports like this one.

lukeelmers
lukeelmers approved these changes May 13, 2020
View reviewed changes
Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Tested bootstrap & check_published_api_changes and all seems to work as expected.

@lukeelmers lukeelmers mentioned this pull request May 13, 2020
Merged
6 tasks
weltenwort
weltenwort approved these changes May 14, 2020
View reviewed changes
Copy link
Member

@weltenwort weltenwort left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work for the infra plugin 👍 Thanks for investing the effort!

@rudolf
Copy link
Contributor Author

rudolf commented May 15, 2020

I'm going to merge without approval from elastic/uptime & elastic/endpoint-app-team since this is blocking #64395 and there is no risk of breaking any behaviour in these changes.

@rudolf rudolf merged commit 795483d into elastic:master May 15, 2020
@rudolf rudolf mentioned this pull request May 15, 2020
Merged
@rudolf rudolf deleted the dont-bootstrap-core-types branch May 15, 2020 09:04
rudolf added a commit to rudolf/kibana that referenced this pull request May 15, 2020
@rudolf
Don't bootstrap core type emits (elastic#66377)
ff36437
rudolf added a commit that referenced this pull request May 15, 2020
@rudolf
Don't bootstrap core type emits (#66377) (#66701)
3a4c86b
jloleysens added a commit to jloleysens/kibana that referenced this pull request May 15, 2020
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into index-managem…
ec822e6 
…ent/add-support-in-url-for-hidden-toggle

* 'master' of github.com:elastic/kibana: (34 commits)
  [SIEM][CASE] Fix bug when connector is deleted. (elastic#65876)
  [SIEM][CASE] Improve layout (elastic#66232)
  [Index Management] Support Hidden Indices (elastic#66422)
  Add Login Selector functional tests. (elastic#65705)
  Lens drilldowns (elastic#65675)
  [ML] Custom template for apiDoc markdown (elastic#66567)
  Don't bootstrap core type emits (elastic#66377)
  [Dashboard] Improve loading error handling (elastic#66372)
  [APM] Minor style fixes for the node strokes (elastic#66574)
  [Ingest Manager] Fix create data source from integration (elastic#66626)
  [Metrics UI] Fix default metric alert interval for new conditions (elastic#66610)
  [Metrics UI] Fix alignment and allow clearing metric value (elastic#66589)
  Don't return package name for non-package data streams (elastic#66606)
  [Ingest Manager] Consolidate routing and add breadcrumbs to all pages (elastic#66475)
  [Docs/Reporting] Have the docs about granular timeout match Cloud docs (elastic#66267)
  Don't automatically add license header to code inside plugins dir. (elastic#66601)
  [APM] Don't trigger map layout if no elements (elastic#66625)
  [Logs UI] Validate ML job setup time ranges (elastic#66426)
  Fix pagination bugs in CCR and Remote Clusters (elastic#65931)
  Add cloud icon for supported settings and embed single-sourced getting started (elastic#65610)
  ...

# Conflicts:
#	x-pack/plugins/index_management/public/application/sections/home/index_list/index_table/index_table.js
#	x-pack/plugins/index_management/server/lib/fetch_indices.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weltenwort weltenwort weltenwort approved these changes

@FrankHassanabad FrankHassanabad FrankHassanabad approved these changes

@lukeelmers lukeelmers lukeelmers approved these changes

@joshdover joshdover joshdover approved these changes

Assignees
No one assigned
Labels
chore release_note:skip Skip the PR/issue when compiling release notes Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.9.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@rudolf @elasticmachine @kibanamachine @weltenwort @FrankHassanabad @lukeelmers @joshdover