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

chore(NA): moving @kbn/apm-utils into bazel #96227

Merged
merged 9 commits into from
Apr 13, 2021
Merged

chore(NA): moving @kbn/apm-utils into bazel #96227

merged 9 commits into from
Apr 13, 2021

Conversation

mistic
Copy link
Member

@mistic mistic commented Apr 5, 2021

One step forward on #69706

That PR moves the @kbn/apm-utils into Bazel by pushing a BUILD file for that package.
After merging this the package will be consumed from within bazel-bin folder.

@mistic mistic added chore Team:Operations Team label for Operations Team v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.13.0 labels Apr 5, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-operations (Team:Operations)

@mistic mistic mentioned this pull request Apr 5, 2021
Closed
23 tasks
@spalger
Copy link
Contributor

spalger commented Apr 5, 2021

Do we have docs on how to work with packages that are built using Bazel yet? Now that we're converting packages I think it's a good time to at least outline how we build and watch for changes in packages, and how to identify which packages have been migrated in the "Getting started" section of the docs.

@mistic mistic requested a review from a team as a code owner April 5, 2021 17:02
@tylersmalley
Copy link
Contributor

Do we have docs on how to work with packages that are built using Bazel yet? Now that we're converting packages I think it's a good time to at least outline how we build and watch for changes in packages, and how to identify which packages have been migrated in the "Getting started" section of the docs.

@mistic, it would be good to run both the legacy build/watch and that of Bazel under the existing kbn build and kbn watch commands. If that is too complex, I would be fine with adding another command. Since most packages are still in lagacy, maybe we add a kbn build-bazel and kbn watch-bazel command. We can either rename them when most are migrated to essentially change the default, or we can drop legacy entirely when all have been migrated. I am hopeful this phase where both exist for packages won't be very long, where I see that being much longer for plugins. If we do this split approach, the default build and watch command should provide notice about the possible need to run the other build and additionally updating the docs to reflect both of these.

cc: @spalger

@spalger
Copy link
Contributor

spalger commented Apr 5, 2021 •
edited
Loading

it would be good to run both the legacy build/watch and that of Bazel under the existing kbn build and kbn watch commands.

If we can detect the BUILD.bazel file and automatically run the right bazel commands from kbn/pm for either the build or watch step that would be amazing. It breaks the existing "cd into the package and run yarn build" expectation but I think we can log the commands we're running when yarn kbn watch -i @kbn/apm-utils is executing and educate users that way, along with a quick doc we can link to about the right commands to run.

@mistic
Copy link
Member Author

mistic commented Apr 6, 2021

@spalger @tylersmalley would you both be okay with just adding 2 new commands for now (yarn kbn build-bazel and yarn kbn watch-bazel) to deal with the bazel built packages and improve and standardise those in the kbn watch and kbn build (with support for -i) once we have none legacy built packages?

@spalger
Copy link
Contributor

spalger commented Apr 6, 2021 •
edited
Loading

If we're implementing the commands why not call them automatically if the package has a BUILD.bazel file? Seems that's the only job that users would have to do and it would be trivial for kbn/pm to just do it for us.

@mistic mistic mentioned this pull request Apr 7, 2021
Merged
@mistic mistic added the auto-backport Deprecated - use backport:version if exact versions are needed label Apr 8, 2021
@mistic
Copy link
Member Author

mistic commented Apr 12, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

✅ unchanged

History

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

jbudz
jbudz approved these changes Apr 13, 2021
View reviewed changes
Copy link
Member

@jbudz jbudz left a comment

Choose a reason for hiding this comment

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

LGTM. I'm assuming the discussion above was moved to #96465, I've tested that separately.

@mistic mistic merged commit 47065ac into elastic:master Apr 13, 2021
kibanamachine added a commit to kibanamachine/kibana that referenced this pull request Apr 13, 2021
@mistic @kibanamachine
chore(NA): moving @kbn/apm-utils into bazel (elastic#96227)
378363b 
* chore(NA): moving @kbn/apm-utils into bazel

* chore(NA): add kbn/apm-utils into package.json

* chore(NA): missing standard on build file globs

* chore(NA): be more explicit about incremental setting

* chore(NA): include pretty in the args for ts_project rule

* docs(NA): include package migration completion in the developer getting started

Co-authored-by: Kibana Machine <[email protected]>
@kibanamachine kibanamachine mentioned this pull request Apr 13, 2021
Merged
@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
✅ 7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit that referenced this pull request Apr 13, 2021
@kibanamachine @mistic
chore(NA): moving @kbn/apm-utils into bazel (#96227) (#97024)
c7f4613 
* chore(NA): moving @kbn/apm-utils into bazel

* chore(NA): add kbn/apm-utils into package.json

* chore(NA): missing standard on build file globs

* chore(NA): be more explicit about incremental setting

* chore(NA): include pretty in the args for ts_project rule

* docs(NA): include package migration completion in the developer getting started

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Tiago Costa <[email protected]>
jloleysens added a commit to jloleysens/kibana that referenced this pull request Apr 14, 2021
@jloleysens
Merge branch 'master' of github.com:elastic/kibana into ilm/support-m…
5b4d940 
…ax_primary_shard_size

* 'master' of github.com:elastic/kibana: (99 commits)
  added missing optional chain for bracket notation (elastic#96939)
  [Discover][DocViewer] Fix toggle columns from doc viewer table tab (elastic#95748)
  [TSVB] Fix per-request caching of index patterns (elastic#97043)
  [Datatable] Fix filter cell flakiness (elastic#96934)
  Unskip heatmap suite and fixes flakiness (elastic#96941)
  [Fleet] Improve performance of data stream API (elastic#97058)
  [ML] Data Frame Analytics: remove beta badge (elastic#96977)
  [App Search] Migrate expanded rows for meta engines table in Engines Overview (elastic#96251)
  Instances latency distribution chart tooltips and axis fixes (elastic#95577)
  [Monitoring] Using primary average shard size (elastic#96177)
  [Workplace Search] Hide Kibana chrome on 3rd party connector redirects (elastic#97028)
  ## [Security Solution] Fixes `Exit full screen` and `Copy to cliboard` styling issues (elastic#96676)
  Index pattern field editor - Add warning on name or type change (elastic#95528)
  [App Search] Add small engine breadcrumb utility helper (elastic#96917)
  Copy esArchiver commands from ./reassign.ts to fix tests (elastic#97012)
  [Security Solution][Detections] Updates MITRE Tactics, Techniques, and Subtechniques for 7.13 (elastic#97011)
  Index patterns server - throw correct error on field caps 404 (elastic#95879)
  Use `EuiThemeProvider` in lists plugin tests and stories (elastic#96129)
  [npm] upgrade caniuse database (elastic#97002)
  chore(NA): moving @kbn/apm-utils into bazel (elastic#96227)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/__jest__/client_integration/edit_policy/serialization/policy_serialization.test.ts
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/form/schema.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jbudz jbudz jbudz approved these changes

@streamich streamich streamich approved these changes

Assignees
No one assigned
Labels
auto-backport Deprecated - use backport:version if exact versions are needed chore release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team v7.13.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@mistic @elasticmachine @spalger @tylersmalley @kibanamachine @jbudz @streamich