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 go code excerpts #4678

Closed
wants to merge 2 commits into from
Closed

Conversation

svrnm
Copy link
Member

@svrnm svrnm commented Jun 13, 2024

I ran npm run codeexcerpts to apply latest changes to the go examples, I think this is one of the first times we do the update, so please take a close look

Signed-off-by: svrnm <neumanns@cisco.com>
@svrnm svrnm requested review from a team June 13, 2024 14:28
@dmathieu
Copy link
Member

This seems wrong, as it removes the log example.

@svrnm
Copy link
Member Author

svrnm commented Jun 13, 2024

This seems wrong, as it removes the log example.

I was surprised about that myself... wait I might have something wrong locally.

Signed-off-by: svrnm <neumanns@cisco.com>
@svrnm svrnm requested a review from a team June 13, 2024 14:50
@@ -409,30 +401,6 @@ func init() {
panic(err)
}
}

func rolldice(w http.ResponseWriter, r *http.Request) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This has been moved to https://github.com/open-telemetry/opentelemetry-go/blob/main/example/dice/rolldice.go so we need to update the documentation accordingly

@svrnm svrnm marked this pull request as draft June 13, 2024 14:55
@svrnm svrnm marked this pull request as draft June 13, 2024 14:55
@svrnm
Copy link
Member Author

svrnm commented Jun 13, 2024

turning this into a draft for now, this needs more thought than I initially anticipated

@chalin
Copy link
Contributor

chalin commented Jun 13, 2024

FYI:

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.

Thanks @svrnm. Btw, this sort of experience goes to show that we can automate the code-snippet refresh (which is super great for minor changes), but when major structural changes are involved, we still need to carefully address the changes in the prose.

/cc @zeitlinger

@zeitlinger
Copy link
Member

Thanks @svrnm. Btw, this sort of experience goes to show that we can automate the code-snippet refresh (which is super great for minor changes), but when major structural changes are involved, we still need to carefully address the changes in the prose.

/cc @zeitlinger

for java, I have created a dedicated doc-snippets project to make this less likely

@svrnm
Copy link
Member Author

svrnm commented Jun 17, 2024

Thanks @svrnm. Btw, this sort of experience goes to show that we can automate the code-snippet refresh (which is super great for minor changes), but when major structural changes are involved, we still need to carefully address the changes in the prose.
/cc @zeitlinger

for java, I have created a dedicated doc-snippets project to make this less likely

Can you share that project?

@zeitlinger
Copy link
Member

Thanks @svrnm. Btw, this sort of experience goes to show that we can automate the code-snippet refresh (which is super great for minor changes), but when major structural changes are involved, we still need to carefully address the changes in the prose.
/cc @zeitlinger

for java, I have created a dedicated doc-snippets project to make this less likely

Can you share that project?

open-telemetry/opentelemetry-java-examples#422

@svrnm
Copy link
Member Author

svrnm commented Jun 18, 2024

Will raise a new PR because of #4679

@svrnm svrnm closed this Jun 18, 2024
@svrnm svrnm mentioned this pull request Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants