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

Link checking fails on external links #182

Closed
bmorelli25 opened this issue Jan 14, 2025 · 7 comments
Closed

Link checking fails on external links #182

bmorelli25 opened this issue Jan 14, 2025 · 7 comments
Assignees
Labels

Comments

@bmorelli25
Copy link
Member

Summary

The build is trying to link check links that don't exist. These links are failing. You can find examples in https://github.com/elastic/observability-docs.md/actions/runs/12758638542/job/35561040730. A couple of examples are below:

Error: `kibana-navigation-search` does not exist in .
    ┌─[/github/workspace/docs/configure-data-sources.md]
    │
 12 │ 1. Find `Logs / Settings` in the [global search field]({{kibana-ref}}/introduction.html#kibana-navigation-search).
    ·  ────────────────────────────────────────────────────────
Error: `privileges-list-cluster` does not exist in .
    ┌─[/github/workspace/docs/synthetics-role-setup.md]
    │
 36 │ | [Cluster]({{ref}}/security-privileges.html#privileges-list-cluster) | `read_pipeline` | Gives the user read-only access to the ingest pipline. |
    ·  ────────────────────────────────────────────────────────

Expected behavior

We can't let these failures prevent the build from succeeding. These links will not work until we move content around and replace this link syntax with the new link syntax/system that Jan is building.

@bmorelli25 bmorelli25 added the bug label Jan 14, 2025
@reakaleek reakaleek self-assigned this Jan 14, 2025
@reakaleek
Copy link
Member

reakaleek commented Jan 14, 2025

The cross-link syntax is already accepted by docs-builder.

Hence, if you add a link like this:

[kibana introduction](kibana://introduction.md)

or

[kibana introduction][kibana-intro]

[kibana-intro]: kibana://introduction.md

Also see #168

Then, the build will succeed.

However, we are not yet resolving the link as it might still depend on the assembler on how it's resolved.

@reakaleek
Copy link
Member

@Mpdreamz

I see two options here:

  • use the new cross-link syntax already
  • ignore links that contain {{ or }} from the check.

WDYT?

@Mpdreamz
Copy link
Member

These are real failures that need addressing but we can make links with {{ or }} in them warnings temporarily.

That way they still get flagged and folks can start addressing them.

e.g ({{ref}}/security-privileges.html#privileges-list-cluster) is a local link that should be written as /security-privileges.md#priveleges-list-cluster.

({{kibana-ref}}/introduction.html#kibana-navigation-search) should be kibana://introduction.md#kibana-navigation-search

The first is something perhaps the migration tool can unroll automatically?

The latter maybe too but requires a more complex mapping of elastic.co urls to repository names.

@reakaleek
Copy link
Member

Ah, I just discovered that {{kibana-ref}} and {{ref}} is defined in https://github.com/elastic/observability-docs.md/blob/main/docs/docset.yml#L494

Does this change anything? @Mpdreamz

@Mpdreamz
Copy link
Member

No we still don't want links like these moving forward.

We don't want variables in links and these links should point to documentation files not URLs.

@reakaleek
Copy link
Member

This will add a warning if the string contains {{ or }} #191

reakaleek added a commit that referenced this issue Jan 14, 2025
Related #182

## Details

Add warning if a link contains `{{` or `}}` thus using a template
expression.

Because 

1) it doesn't work
2) it's discouraged to use template variables in links
@reakaleek
Copy link
Member

I this solved with #191?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants