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

Add asciidoc support for generated plugin list #72292

Conversation

stacey-gammon
Copy link
Contributor

@stacey-gammon stacey-gammon commented Jul 17, 2020

Adds README asciidoc support so READMEs in asciidoc are now part of the developer guide:

Screen Shot 2020-08-05 at 10 02 56 AM

Screen Shot 2020-08-05 at 10 03 01 AM

Also:

  • Rearranged the docs because there is a limit of 3 levels deep in docs. Instead of Developer Guide -> Architecture -> Code exploration -> Plugin Readmes it's now Developer Guide -> Plugin list -> Plugin READMEs. I think it's more discoverable there anyway.

Screen Shot 2020-08-05 at 9 58 56 AM

  • Change the name and contents of the file. The text about the folder hierarchy best practices didn't seem to go in there and just made a long page longer. Now it's just a table of plugins.

  • Made it a table for better readability:

Before:
Screen Shot 2020-08-05 at 10 01 39 AM

After:
Screen Shot 2020-08-05 at 9 46 55 AM

@stacey-gammon stacey-gammon force-pushed the 2020-07-17-asciidoc-support-for-plugin-list branch 3 times, most recently from 0edf151 to ae995aa Compare July 21, 2020 15:57
@stacey-gammon stacey-gammon force-pushed the 2020-07-17-asciidoc-support-for-plugin-list branch 2 times, most recently from 45466c9 to a8d024d Compare July 28, 2020 17:44
@gtback
Copy link
Member

gtback commented Jul 28, 2020

@elasticmachine run elasticsearch-ci/docs

@stacey-gammon stacey-gammon force-pushed the 2020-07-17-asciidoc-support-for-plugin-list branch from a8d024d to a1131b2 Compare July 28, 2020 18:48
@gtback
Copy link
Member

gtback commented Jul 28, 2020

@elasticmachine run elasticsearch-ci/docs

With any luck this will finally pass!

@stacey-gammon stacey-gammon force-pushed the 2020-07-17-asciidoc-support-for-plugin-list branch 4 times, most recently from 1d4a2d0 to 9beaa8a Compare July 29, 2020 14:30
@stacey-gammon
Copy link
Contributor Author

@spalger - any idea why this would fail with "js file outside regularBundlesPath"?

@spalger
Copy link
Contributor

spalger commented Jul 29, 2020

That looks a lot like an Angular provider converted to JSON... { '$$id': 4, apply: [Function], call: [Function] } 🤔

@stacey-gammon stacey-gammon force-pushed the 2020-07-17-asciidoc-support-for-plugin-list branch from 9beaa8a to 4bb7f5a Compare July 29, 2020 18:08
@stacey-gammon
Copy link
Contributor Author

I'll sync with master again and 🤞

@spalger
Copy link
Contributor

spalger commented Jul 29, 2020

Looks like it's opal-runtime, which is installed by asciidoctor and provides a runtime to run ruby code that's transformed to JS... This seems like an unnecessary amount of impact for a text parser, and the fact that it's injecting globals and breaking tests is pretty concerning. Is there another dependency we can use besides asciidoctor? If not, maybe we can consume it through the CLI rather than via the node API?

@stacey-gammon stacey-gammon force-pushed the 2020-07-17-asciidoc-support-for-plugin-list branch 2 times, most recently from 2d6c609 to b83d12a Compare July 31, 2020 15:53
@stacey-gammon
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/docs

@gtback
Copy link
Member

gtback commented Aug 3, 2020

@stacey-gammon looks like all tests are passing 🙌 !

@stacey-gammon
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/docs

Thanks @gtback! I lost track of this PR for a couple days. :)

Try level offset "=+2" instead of "=+1" to stop the inlining of the includes.

remove +2 back to +1
@stacey-gammon stacey-gammon force-pushed the 2020-07-17-asciidoc-support-for-plugin-list branch from b83d12a to acc37d9 Compare August 3, 2020 13:32
@stacey-gammon stacey-gammon added review Team:Operations Team label for Operations Team labels Aug 5, 2020
@elasticmachine
Copy link
Contributor

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

@spalger
Copy link
Contributor

spalger commented Aug 5, 2020

@stacey-gammon when we make changes to the readme files, and then in turn update the plugin list in the docs, are we verifying the syntax is valid in CI? (via elasticsearch-ci/docs I assume)

Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

Removal of the empty grokdebugger README LGTM.

@gtback
Copy link
Member

gtback commented Aug 5, 2020

@spalger yes, any of the Asciidoc files that get included in the build process will be validated by elasticsearch-ci/docs which rebuilds documentation for any of the files that have changed.

Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

LGTM, one little totally ignorable nitpick

Comment on lines +37 to +49
const { firstParagraph, anchor } = extractAsciidocInfo(
`[[this-is-the-anchor]]
== Heading here

Intro.

=== Another heading

More details`
);

expect(firstParagraph).toEqual(`Intro.`);
expect(anchor).toEqual('this-is-the-anchor');
Copy link
Contributor

@spalger spalger Aug 5, 2020

Choose a reason for hiding this comment

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

I'm sure you've got your reasons for not preferring this, but wanted to suggest considering inline snapshots

import dedent from 'dedent';

//...

expect(
  extractAsciidocInfo(dedent`
    [[this-is-the-anchor]]
    == Heading here

    Intro.

    === Another heading

    More details
  `)
).toMatchInlineSnapshot(...autogenerated by jest...)

@stacey-gammon
Copy link
Contributor Author

Thought I merged this weeks ago 🤦‍♀️ .

[[dashboard-enhanced-plugin]]
== Dashboard app enhancements plugin

Adds drilldown capabailities to dashboard. Owned by the Kibana App team.
Copy link
Member

Choose a reason for hiding this comment

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

Seems to be a typo here (capabailities):

Suggested change
Adds drilldown capabailities to dashboard. Owned by the Kibana App team.
Adds drilldown capabilities to dashboard. Owned by the Kibana App team.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thanks for the review!

Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Apart from a minor typo, KibanaApp owned code LGTM. didn't test.

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Build metrics

✅ unchanged

History

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

@stacey-gammon stacey-gammon merged commit 4ffef0a into elastic:master Aug 24, 2020
stacey-gammon added a commit to stacey-gammon/kibana that referenced this pull request Aug 24, 2020
* add asciidoc support for generated plugin list

Try level offset "=+2" instead of "=+1" to stop the inlining of the includes.

remove +2 back to +1

* Remove asciidoc, switch to regex. Rearrange dev guide to avoid nesting limit.

* Add tests for regex

* add a description to not throw off the table. Remove the heading from the paragraph snippet.

* Fix more READMEs so table renders correctly

* Update plugin list

* Remove code-exploration file, moved to plugin-list

* fix typo

* Add link to developer examples

* Update plugin list

* fix typo
# Conflicts:
#	docs/developer/architecture/code-exploration.asciidoc
stacey-gammon added a commit to stacey-gammon/kibana that referenced this pull request Aug 24, 2020
* add asciidoc support for generated plugin list

Try level offset "=+2" instead of "=+1" to stop the inlining of the includes.

remove +2 back to +1

* Remove asciidoc, switch to regex. Rearrange dev guide to avoid nesting limit.

* Add tests for regex

* add a description to not throw off the table. Remove the heading from the paragraph snippet.

* Fix more READMEs so table renders correctly

* Update plugin list

* Remove code-exploration file, moved to plugin-list

* fix typo

* Add link to developer examples

* Update plugin list

* fix typo
# Conflicts:
#	docs/developer/architecture/code-exploration.asciidoc
stacey-gammon added a commit that referenced this pull request Aug 24, 2020
* add asciidoc support for generated plugin list

Try level offset "=+2" instead of "=+1" to stop the inlining of the includes.

remove +2 back to +1

* Remove asciidoc, switch to regex. Rearrange dev guide to avoid nesting limit.

* Add tests for regex

* add a description to not throw off the table. Remove the heading from the paragraph snippet.

* Fix more READMEs so table renders correctly

* Update plugin list

* Remove code-exploration file, moved to plugin-list

* fix typo

* Add link to developer examples

* Update plugin list

* fix typo
# Conflicts:
#	docs/developer/architecture/code-exploration.asciidoc
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Aug 26, 2020
@kibanamachine
Copy link
Contributor

Looks like this PR has backport PRs but they still haven't been merged. Please merge them ASAP to keep the branches relatively in sync.

@stacey-gammon stacey-gammon removed the backport missing Added to PRs automatically when the are determined to be missing a backport. label Aug 26, 2020
parkiino pushed a commit to parkiino/kibana that referenced this pull request Sep 1, 2020
* add asciidoc support for generated plugin list

Try level offset "=+2" instead of "=+1" to stop the inlining of the includes.

remove +2 back to +1

* Remove asciidoc, switch to regex. Rearrange dev guide to avoid nesting limit.

* Add tests for regex

* add a description to not throw off the table. Remove the heading from the paragraph snippet.

* Fix more READMEs so table renders correctly

* Update plugin list

* Remove code-exploration file, moved to plugin-list

* fix typo

* Add link to developer examples

* Update plugin list

* fix typo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported release_note:skip Skip the PR/issue when compiling release notes review Team:Operations Team label for Operations Team v7.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants