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

[infra] Factor out include partial from shortcode #6494

Conversation

chalin
Copy link
Contributor

@chalin chalin commented Mar 7, 2025

  • Refactoring in prep for [i18n] Factor prose out of shortcodes like docs/latest-release #6460 and other work
  • Why this change: we can't call shortcodes from other shortcodes or partials (without resorting to .RenderString magic, which doesn't work when we're trying to keep things all in markdown).
  • Factors out the core functionality of the includes.html shortcode and makes it available as a partial of the same name
  • In prep for 6460, this refactors the docs/languages/index-intro.md shortcode for better separate concerns: have the natural language part in an include file, while keeping the code in a shortcode
    • An include-file version of the docs/languages/index-intro.md shortcode
    • A new version of the shortcode (suffixed with "2"), which isn't currently used

The only change in the generated site files are for pages with includes. Any included text with an apostrophe (') is now encoded as ’:

$ (cd public && g diff -bw --ignore-blank-lines -I "'|’") | grep ^diff | head
diff --git a/site/index.html b/site/index.html
diff --git a/sitemap.xml b/sitemap.xml

Preview: https://deploy-preview-6494--opentelemetry.netlify.app/docs/languages/cpp/instrumentation/

Here's what the diff looks like:

image

@chalin chalin added CI/infra CI & infrastructure cleanup/refactoring i18n Internationalization and localization labels Mar 7, 2025
@chalin chalin requested a review from a team as a code owner March 7, 2025 18:05
@chalin chalin changed the title [infra] Factor include partial from shortcode [infra] Factor out include partial from shortcode Mar 7, 2025
Copy link
Contributor

@tiffany76 tiffany76 left a comment

Choose a reason for hiding this comment

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

RSLGTM, with one non-blocking question.

---
---

This is the OpenTelemetry {{ $name }} documentation. OpenTelemetry is an
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this introduction is how it's been done for ages, but I find "This is" a bit weak, like it points out the obvious. Maybe we could say something like "Welcome to the OpenTelemetry (name) documentation...."?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. Let's tackle that change in a followup PR.

@chalin chalin merged commit 3c38c33 into open-telemetry:main Mar 7, 2025
18 checks passed
@chalin chalin deleted the chalin-im-rw-includes-refactoring-2025-03-07 branch March 7, 2025 19:26
ymotongpoo pushed a commit to ymotongpoo/opentelemetry.io that referenced this pull request Mar 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/infra CI & infrastructure cleanup/refactoring i18n Internationalization and localization
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants