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.

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

[Code] Embedded Code Snippet Component #47183

Merged
merged 2 commits into from
Oct 5, 2019

Conversation

rylnd
Copy link
Contributor

@rylnd rylnd commented Oct 3, 2019

Summary

This addresses elastic/code#1635.

  • Adds new route for integrations work, hidden behind a config var (xpack.code.integrations.enabled)
  • Adds mock data to simulate what API consumers would be passing to these integration components
  • Updates CodeBlock component to minimize the API and make it more general
    • Adds defaultProps as appropriate to remove boilerplate
    • Minor style tweaks on the existing CodeBlock component

Kibana

Checklist

Use strikethroughs to remove checklist items you don't feel are applicable to this PR.

For maintainers

@rylnd rylnd added Team:Code release_note:skip Skip the PR/issue when compiling release notes v7.6.0 labels Oct 3, 2019
@rylnd rylnd requested a review from a team as a code owner October 3, 2019 00:13
@elasticmachine
Copy link
Contributor

Pinging @elastic/code (Team:Code)

@rylnd
Copy link
Contributor Author

rylnd commented Oct 3, 2019

@mw-ding same caveats here as mentioned in #47180 (comment).

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

Copy link
Contributor

@mw-ding mw-ding left a comment

Choose a reason for hiding this comment

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

I skipped the part for the import selector and I am going to comment on the other PR.

For this one, I leave some minor comments inline. I think let's target on finalizing the EmbeddedCodeBlock interface and send out for review today or tomorrow?

* you may not use this file except in compliance with the Elastic License.
*/

export interface Snippet {
Copy link
Contributor

Choose a reason for hiding this comment

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

This would be the eventual API that APM team will pass through. Shall we export these type definitions and the EmbeddedCodeBlock component to code/public/index.ts as we have chatted?

Copy link
Contributor

Choose a reason for hiding this comment

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

also, we probably need to provide a util function to convert APM's code context data into this shape. I will paste around a sample APM stacktrace data here later.

@@ -61,6 +62,9 @@ export const code = (kibana: any) =>
ui: Joi.object({
enabled: Joi.boolean().default(true),
}).default(),
integrations: Joi.object({
Copy link
Contributor

Choose a reason for hiding this comment

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

please rebase because of this change: #46664. For this flag, just leave here as the xpack.code.integrations.enabled flag does.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was rebased onto master; it wasn't obvious what needed to change and there were no conflicts, so nothing really changed here. Please give this another look.

Copy link
Contributor

Choose a reason for hiding this comment

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

It should be OK then.

@elasticmachine
Copy link
Contributor

💔 Build Failed

* Adds new route hidden behind a config var
(`xpack.code.integrations.enabld`)
* Updates CodeBlock component to have a smaller API surface
* Adds several example components for various pieces of the new APM
integration designs.
@rylnd rylnd mentioned this pull request Oct 4, 2019
7 tasks
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Copy link
Contributor

@mw-ding mw-ding left a comment

Choose a reason for hiding this comment

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

LGTM.

Hits and ranges were both part of our search results queries; O11y won't
have that information.
@mw-ding
Copy link
Contributor

mw-ding commented Oct 4, 2019

@daveyholler this one as well. :)

@elasticmachine
Copy link
Contributor

💔 Build Failed

@rylnd
Copy link
Contributor Author

rylnd commented Oct 4, 2019

retest

@rylnd rylnd requested a review from daveyholler October 4, 2019 23:34
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@rylnd rylnd merged commit c505fcb into elastic:master Oct 5, 2019
@rylnd rylnd deleted the code-integrations-snippet branch October 5, 2019 00:02
rylnd added a commit to rylnd/kibana that referenced this pull request Oct 5, 2019
* Add Integrations page and POC CodeBlock usage

* Adds new route hidden behind a config var
(`xpack.code.integrations.enabld`)
* Updates CodeBlock component to have a smaller API surface
* Adds several example components for various pieces of the new APM
integration designs.

* Pare down stub data

Hits and ranges were both part of our search results queries; O11y won't
have that information.
jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 7, 2019
… into console-token-iterator

* 'console-token-iterator' of github.com:jloleysens/kibana: (184 commits)
  [functional/services] update webdriver lib and types (elastic#47381)
  Standardizing IconField implementation across the app (elastic#47196)
  Move ui/value_suggestions ⇒ NP data plugin (elastic#45762)
  Remove ui/persisted_log - Part 2 (elastic#47236)
  Update gulp related packages (elastic#47421)
  Update dependency idx to ^2.5.6 (elastic#47399)
  try running fewer jobs in parallel on the same worker (elastic#47403)
  Update webpack related packages (elastic#47402)
  Update jsonwebtoken related packages (elastic#47400)
  Update gulp related packages (major) (elastic#46665)
  Update dependency prettier to ^1.18.2 (elastic#47340)
  Update dependency @types/puppeteer to ^1.20.1 (elastic#47339)
  Update dependency @elastic/elasticsearch to ^7.4.0 (elastic#47338)
  Update dependency tar-fs to ^1.16.3 (elastic#47341)
  [Code] Code Integrator Component (elastic#47180)
  [Canvas][i18n] Sidebar (elastic#46090)
  Generate uuid in task Manager as Kibana uuid may not yet have been initialised
  [Code] Embedded Code Snippet Component (elastic#47183)
  Revert "Add pipeline for flaky test runner job (elastic#46740)"
  SearchSource: fix docvalue_fields and fields intersection logic (elastic#46724)
  ...
rylnd added a commit that referenced this pull request Oct 7, 2019
* Add Integrations page and POC CodeBlock usage

* Adds new route hidden behind a config var
(`xpack.code.integrations.enabld`)
* Updates CodeBlock component to have a smaller API surface
* Adds several example components for various pieces of the new APM
integration designs.

* Pare down stub data

Hits and ranges were both part of our search results queries; O11y won't
have that information.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes v7.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants