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

Update navigation.yml #780

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Update navigation.yml #780

wants to merge 8 commits into from

Conversation

colleenmcginnis
Copy link
Contributor

Copy link

Label error. Requires exactly 1 of: automation, breaking, bug, changelog:skip, chore, ci, dependencies, documentation, enhancement, feature, fix, redesign. Found:

Copy link
Contributor Author

@colleenmcginnis colleenmcginnis left a comment

Choose a reason for hiding this comment

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

Some specific questions below!

Comment on lines 295 to 301
# I'm not sure what's going on with this one because there is no
# toc.yml in reference/elasticsearch/data-analysis/* on `main`.
# Currently these pages are captured in the higher level reference/toc.yml.
# In https://github.com/elastic/elasticsearch/pull/125118 I'm proposing we remove
# reference/toc.yml and distribute the contents across 9 individual toc.yml files.
- toc: elasticsearch://reference/data-analysis
path_prefix: reference/elasticsearch/data-analysis
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Question for @Mpdreamz...

Copy link
Member

Choose a reason for hiding this comment

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

This was added because neither elasticsearch://reference was not linked anywhere and thus no prefix matched elasticsearch://reference/data-analysis.

Let's revisit once: elastic/elasticsearch#125118 lands

Comment on lines 302 to 306

# This makes sense for now. I'm not sure when these files will be
# moved to https://github.com/elastic/detection-rules.
- toc: security-docs://reference/prebuilt-rules
path_prefix: reference/prebuilt-rules
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 can be moved up to replace lines 161-162.

Copy link
Member

Choose a reason for hiding this comment

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

Comment on lines 427 to 440
# Commenting out because these don't exist yet!!
# 📝 TO DO: Get @karenzone's thoughts
#
# # Logstash plugins (LSR)
# # 📝 TO DO: Migrate all files and create toc.yml
# - toc: logstash-docs-md://lsr
# path_prefix: reference/logstash/plugins
# # Children include the entire AsciiDoc book

# # Logstash versioned plugins (VPR)
# # 📝 TO DO: Migrate all files and create toc.yml
# - toc: logstash-docs-md://vpr
# path_prefix: reference/logstash/versioned-plugins
# # Children include the entire AsciiDoc book
Copy link
Contributor Author

@colleenmcginnis colleenmcginnis Mar 19, 2025

Choose a reason for hiding this comment

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

Question for @karenzone on logstash path_prefixes. cc @bmorelli25

Comment on lines +444 to +450
# 📝 TO DO: Answer the question below
# This is the first URL that has "serverless" in the path. It's
# https://www.elastic.co/guide/en/esf/current/aws-elastic-serverless-forwarder.html
# so not really the first thing we want folks to land on if path has a lot of impact
# on search results. @KOTungseth WDYT?
- toc: elastic-serverless-forwarder://reference
path_prefix: reference/elastic-serverless-forwarder
path_prefix: reference/aws-forwarder
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Question for @KOTungseth.

@colleenmcginnis
Copy link
Contributor Author

FYI @Mpdreamz we will need to merge elastic/docs-content#792 to get the final structure of the global nav.

Copy link
Contributor Author

@colleenmcginnis colleenmcginnis left a comment

Choose a reason for hiding this comment

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

Ok. I think this is pretty stable now. I left a few comments below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hope it's ok to update these for the repos where the migration PRs have merged. I kept skip: true for the repos that use master because I got an error on build-all when I tried to include them.

Copy link
Member

Choose a reason for hiding this comment

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

<repo>:
    current: master

Should work (like cloud).

Copy link
Member

Choose a reason for hiding this comment

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

(current/next) not yet supported so we just do current now with current defaulting to main

Comment on lines +76 to 77
logstash-docs-md:
skip: true
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 content is still in flux.

Comment on lines +330 to 338
# 📝 TO DO: Delete this item
# This should be replaced by `elasticsearch://reference/enrich-processor` below
- toc: elasticsearch://reference/ingestion-tools/enrich-processor
path_prefix: reference/elasticsearch/enrich-processor

# Processor reference
# 📝 TO DO: Directory depth in elasticsearch repo: would require toc 2 levels down
# 🔜 https://github.com/colleenmcginnis/elasticsearch/blob/docs-assembler-prep/docs/reference/enrich-processor/toc.yml
# ✅ https://github.com/elastic/elasticsearch/blob/main/docs/reference/enrich-processor/toc.yml
- toc: elasticsearch://reference/enrich-processor
path_prefix: reference/enrich-processor
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would expect we'd be able to remove elasticsearch://reference/ingestion-tools/enrich-processor in favor of elasticsearch://reference/enrich-processor now that elastic/elasticsearch#125118 was merged, but at least when I run build-all locally I get errors if I delete this.

Comment on lines +371 to 379
# 📝 TO DO: Delete this item
# This should be replaced by `elasticsearch://reference/search-connectors` below
- toc: elasticsearch://reference/ingestion-tools/search-connectors
path_prefix: reference/elasticsearch/search-connectors

# Search connectors
# 📝 TO DO: Directory depth in elasticsearch repo: would require toc 2 levels down
# 🔜 https://github.com/colleenmcginnis/elasticsearch/blob/docs-assembler-prep/docs/reference/search-connectors/toc.yml
# ✅ https://github.com/elastic/elasticsearch/blob/main/docs/reference/search-connectors/toc.yml
- toc: elasticsearch://reference/search-connectors
path_prefix: reference/search-connectors
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would expect we'd be able to remove elasticsearch://reference/ingestion-tools/search-connectors in favor of elasticsearch://reference/search-connectors below now that elastic/elasticsearch#125118 was merged, but at least when I run build-all locally I get errors if I delete this.

Comment on lines +493 to 504
# 📝 TO DO: Delete this item
# This should be replaced by `elasticsearch://reference/text-analysis` and
# `elasticsearch://reference/aggregations` below.
- toc: elasticsearch://reference/data-analysis
path_prefix: reference/elasticsearch/data-analysis

# ✅ https://github.com/elastic/elasticsearch/blob/main/docs/reference/text-analysis/toc.yml
- toc: elasticsearch://reference/text-analysis
path_prefix: reference/text-analysis
# 📝 TO DO: Directory depth in elasticsearch repo: would require toc 2 levels down
# 🔜 https://github.com/colleenmcginnis/elasticsearch/blob/docs-assembler-prep/docs/reference/aggregations/toc.yml
# ✅ https://github.com/elastic/elasticsearch/blob/main/docs/reference/aggregations/toc.yml
- toc: elasticsearch://reference/aggregations
path_prefix: reference/aggregations
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would expect we'd be able to remove elasticsearch://reference/data-analysis in favor of elasticsearch://reference/text-analysis and elasticsearch://reference/aggregations below now that elastic/elasticsearch#125118 was merged, but at least when I run build-all locally I get errors if I delete this.

Copy link
Member

@Mpdreamz Mpdreamz Mar 21, 2025

Choose a reason for hiding this comment

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

Hey @colleenmcginnis you might need to delete your .artifacts/checkouts folder and call clone-all again.

Alternatively you should be able to call clone-all without deleting to update all but I have not tested if it works in all scenario's yet.

@colleenmcginnis colleenmcginnis marked this pull request as ready for review March 21, 2025 18:35
@colleenmcginnis colleenmcginnis requested a review from a team as a code owner March 21, 2025 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants