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

feat: [es] Create docs/languages/js/_index.md #5327

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

Conversation

oayras
Copy link

@oayras oayras commented Oct 5, 2024

Related issue #5229

@oayras oayras requested a review from a team as a code owner October 5, 2024 06:38
Copy link

linux-foundation-easycla bot commented Oct 5, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@opentelemetrybot opentelemetrybot requested a review from a team October 5, 2024 06:38
@opentelemetrybot opentelemetrybot requested review from a team and krol3 and removed request for a team October 5, 2024 06:38
Copy link
Member

@theletterf theletterf left a comment

Choose a reason for hiding this comment

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

¡Gracias! Un par de comentarios tan solo.

content/es/docs/languages/js/_index.md Outdated Show resolved Hide resolved
content/es/docs/languages/js/_index.md Outdated Show resolved Hide resolved
@theletterf
Copy link
Member

+CC @krol3

@opentelemetrybot
Copy link
Collaborator

You triggered fix:all action run at https://github.com/open-telemetry/opentelemetry.io/actions/runs/11659573015

@opentelemetrybot
Copy link
Collaborator

fix:all was successful.

IMPORTANT: (RE-)RUN /fix:all to ensure that there are no remaining check issues.

@theletterf theletterf added this pull request to the merge queue Nov 4, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 4, 2024
@oayras
Copy link
Author

oayras commented Nov 6, 2024

default_lang_commit value was fixed

Copy link
Contributor

Choose a reason for hiding this comment

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

Let me investigate if we can achieve the same result without introducing an extra parameter.

@opentelemetrybot opentelemetrybot requested review from a team November 13, 2024 11:15
@chalin
Copy link
Contributor

chalin commented Nov 13, 2024

This PR needs to be rebased. /cc @theletterf

@@ -3,25 +3,26 @@
<!-- prettier-ignore -->
` -}}
{{ $lang := .Get 0 -}}
{{ $translation := .Get 1 -}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Please accept the following change, and then revert all of the "en" argument additions to the en pages:

Suggested change
{{ $translation := .Get 1 -}}
{{ $translation := .Get 1 | default "en" -}}

@chalin chalin force-pushed the docs/languages/index branch from b049432 to d33fc6d Compare November 13, 2024 11:32
@opentelemetrybot opentelemetrybot requested review from a team November 13, 2024 11:32
@chalin
Copy link
Contributor

chalin commented Nov 13, 2024

This PR needs to be rebased. /cc @theletterf

I've rebased and resolved conflicts as best I could. PTAL

@chalin
Copy link
Contributor

chalin commented Nov 13, 2024

@theletterf et all: in general, please tag me on localization PRs that have such a "broad" (cross-localization) impact and/or changes to shared layouts so that we can together assess and discuss design alternatives.

Hugo has built-in support for i18n, in the way that you've implemented it here. So we don't need to re-implement our own localization tag support.

I need to think about this a bit more before I can make some concrete suggestions. Stay tuned!

Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

After thinking about this more, for the moment, I'd rather that each locale create it's own version of the shortcode, like this: layouts/shortcodes/pt/docs/languages/index-intro.md.

There are pros and cons to this approach, vs. the i18n-tag approach you've suggested here (which, if we were to adopt, I'd suggest rewriting so that we use Hugo's native support). For now, I'm voting for the per-locale shortcode approach, but I'm open to discussing this more with @open-telemetry/docs-es-approvers if they feel strongly about the alternative.

@open-telemetry/docs-approvers - please also share your opinion.


The current status of the major functional components for OpenTelemetry
{{ $name }} is as follows:
{{ $traces := $content.introPage.releases.traces }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{{ $traces := $content.introPage.releases.traces }}
{{ $traces := $content.introPage.releases.traces }}

@oayras gracias por tu contribucion, seria un abordage mas simples, traduzir el texto, sin necesitad de agregar nuevos archivos en data/languages/en.yaml. Por favor intentar este abordage, y podemos hacer un zoom. Por favor escribeme en el slack

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

9 participants