-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 payment.md code sample to require
./index.js
#4256
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@open-telemetry/demo-approvers PTAL
This is not correct. What is missing is how you require this, which is done as part of the node command to start the app itself.
|
|
@marleypowell @open-telemetry/demo-approvers any more updates on this PR? |
@marleypowell @open-telemetry/demo-approvers PTAL |
This PR in its current form, is not correct. The docs today are also not correct. See this comment on what the proper solution is, which would also require rewriting the paragraph description on the docs page: #4256 (comment) |
The description paragraph mentions: > It then requires your app at ./index.js to start it up once the SDK is initialized. This seems to be missing from the code sample.
thanks. @marleypowell can you fix the PR accordingly? |
I've updated it based on the suggestion 🙂 |
The description paragraph mentions:
https://github.com/open-telemetry/opentelemetry.io/blob/9b9596e21766406b0d93b203fd1bf184f831111e/content/en/docs/demo/services/payment.md?plain=1#L21C1-L25C24
This seems to be missing from the code sample.