Skip to content

[DOC] Fix bad link to resolve docs-ci failure #166

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

Closed
wants to merge 4 commits into from
Closed

[DOC] Fix bad link to resolve docs-ci failure #166

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
62873ae
[DOC] Fix bad link to resolve docs-ci failures
karenzone Feb 24, 2023
ad50192
Remove add'l formatting errors
karenzone Feb 24, 2023
b79fe89
Move descriptive text outside of link
karenzone Mar 20, 2023
5b0e0c6
Merge branch 'main' into karenzone-fix-link
karenzone Mar 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/index.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ This plugin supports the following configuration options plus the <<plugins-{typ
| <<plugins-{type}s-{plugin}-query_template>> |<<string,string>>|No
| <<plugins-{type}s-{plugin}-result_size>> |<<number,number>>|No
| <<plugins-{type}s-{plugin}-retry_on_failure>> |<<number,number>>|No
| <<plugins-{type}s-{plugin}-result_on_status_>> |<<number,number list>>|No
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yaauie, please validate that this fix aligns with your original intent before I merge it.

| <<plugins-{type}s-{plugin}-retry_on_status>> |<<number,number>>|No
Copy link
Contributor

Choose a reason for hiding this comment

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

The intent here was to have the text of the link say "number list" because it takes a list of numbers.

Copy link
Contributor Author

@karenzone karenzone Mar 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

What about result_on_status_>> --> retry_on_status>>? This change aligns with labeling in descriptions section, but I don't want to guess incorrectly.

Copy link
Contributor Author

@karenzone karenzone Mar 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

<<number,number list>> will work for plugins generated for the Logstash Plugin Reference, but will fail in the Versioned Plugin Reference (VPR). Why? Because the abbreviated anchor format (<<internal-anchor>>) works only within the same book. Outside of the context of a particular book, that format has no meaning.

  • LSR. This format works in the LSR because the referenced topic is present in the Logstash Reference.

  • VPR. The reason the format for our typical Value types works in the VPR at all is because of gsubs in the VPR docgen script:

    Screen Shot 2023-03-10 at 2 40 32 PM

So when you modify the expected format to be <<number,number list>>, it doesn't match the gsub <<number,number>>. It renders literally and fails docs CI in the VPR docgen PR.

Copy link
Contributor Author

@karenzone karenzone Mar 10, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Related:
If a number list is something we expect to use more frequently in the future, we should update docs for to add number list as a value type. OR expand the current entry for number to show an example/description of a number list. The current description and example is for a single entry.

Copy link
Contributor

Choose a reason for hiding this comment

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

🤔 -> elastic/docs-tools#86

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure we want to make it so easy to deviate from the standard. Instead we should vet and update links and gsubs to be more appropriate to our input types.

If a number list is something we expect to use more frequently in the future, we should update docs for to add number list as a value type. OR expand the current entry for number to show an example/description of a number list. The current description and example is for a single entry.

| <<plugins-{type}s-{plugin}-sort>> |<<string,string>>|No
| <<plugins-{type}s-{plugin}-ssl>> |<<boolean,boolean>>|No
| <<plugins-{type}s-{plugin}-keystore>> |a valid filesystem path|No
Expand Down