Skip to content

Conversation

@cartermp
Copy link
Contributor

This adds the Ruby version of open-telemetry/opentelemetry.io#1808

Copy link
Contributor

@plantfansam plantfansam left a comment

Choose a reason for hiding this comment

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

LGTM

@cartermp
Copy link
Contributor Author

@svrnm and @chalin PTAL

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.

LGTM. Have you checked that this builds and renders properly?

@cartermp
Copy link
Contributor Author

Yeah, this is how it renders when it's built locally:

image

On my machine the redirect rules aren't applied (I used npm run serve) and so the link doesn't apply to redirect there...is there a way to verify redirects locally?

@chalin
Copy link
Contributor

chalin commented Oct 27, 2022

On my machine the redirect rules aren't applied (I used npm run serve) and so the link doesn't apply to redirect there...is there a way to verify redirects locally?

The netlify-cli can be finicky and flakey but it is getting much better. Redirect rules work most of the time for me now (consider running npm install if you haven't done so in a while). Sometimes if you leave the server running, it is best to stop and restart it after making redirect-rule changes.

@arielvalentin
Copy link
Contributor

@chalin @cartermp et.al. I moved these docs into this repo some time ago because I was following patterns set by other sigs, but it feels like more toil to have to update both repos.

Would it help to move these docs back into opentelemetry.io?

@cartermp
Copy link
Contributor Author

@arielvalentin yes! Heh. In fact, most languages are moved back to the website repo. I believe that part of the reason for the split was because it was earlier days where literally nothing was stable and even the tracing SDKs were changing every day. Now that most languages are GA for tracing and some degree of stable for metrics, there's less churn in defining how you even use an SDK, and a lot more an adding scenarios, commentary, and general docs completeness.

I was planning on bringing this to the Ruby SIG next week or the week after, but if it's acceptable to just do the move now we can definitely accommodate. There's some more things to consider that may still be good to chat about (e.g., how to eventually roll out metrics docs, where API docs should live, something something logs signal, etc.) - more than happy to chat about it.

@arielvalentin
Copy link
Contributor

Thank you very much for your help here with maintaining docs!

I was planning on bringing this to the Ruby SIG next week or the week after, but if it's acceptable to just do the move now we can definitely accommodate. There's some more things to consider that may still be good to chat about (e.g., how to eventually roll out metrics docs, where API docs should live, something something logs signal, etc.) - more than happy to chat about it.

I am in favor of moving the docs! My one request is that @open-telemetry/ruby-approvers and @open-telemetry/ruby-contrib-approvers be granted collaborator access and added to CODEOWNERS to the ruby docs in repo so that we are able to approve PRs.

@chalin
Copy link
Contributor

chalin commented Oct 27, 2022

I am in favor of moving the docs!

Wonderful!

My one request is that @open-telemetry/ruby-approvers and @open-telemetry/ruby-contrib-approvers be granted collaborator access and added to CODEOWNERS to the ruby docs in repo so that we are able to approve PRs.

Yes, of course.

I'll create an issue to track the necessary work, as we did for the other language SIGs.

@arielvalentin - Can we get this PR merged soon (before the move)?

@arielvalentin
Copy link
Contributor

@chalin absolutely! Sadly I am not a maintainer in this repo...

@robertlaurin robertlaurin merged commit 32f4a7a into open-telemetry:main Oct 28, 2022
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.

6 participants