-
Notifications
You must be signed in to change notification settings - Fork 448
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
Add Useful Resources and Example Build Instructions to CONTRIBUTING.md #97
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.
Thanks for putting this together!
I wonder about merging this with the CONTRIBUTING.md. There's quite a bit of information that is duplicated in both documents, so that makes it harder to maintain when things change. I think most parts of this guide could be added under a Useful resources section in the CONTRIBUTING.md.
Especially for newcomers, it would be great if we had a quick start guide similar to Java's, and links to relevant spec parts from there.
+1 |
Thanks for the feedback! I am working with Ankit who made PR 94 to move our documentation into CONTRIBUTING.md. |
* Read through the [Open Telemetry Collector](https://github.com/open-telemetry/opentelemetry-collector) GitHub repository | ||
* This repository has a lot of good information surrounding the Open Telemetry ecosystem. At the top of the **[readme](https://github.com/open-telemetry/opentelemetry-collector/blob/master/README.md)**, there are multiple links that give newcomers a good idea of what the project is about and how to get involved in it. | ||
* Read through the Open Telemetry Python documentation for its [API](https://opentelemetry-python.readthedocs.io/en/stable/api/api.html) and [SDK](https://opentelemetry-python.readthedocs.io/en/stable/sdk/sdk.html) | ||
* The [API](https://github.com/open-telemetry/opentelemetry-java/blob/master/QUICKSTART.md) and [SDK](https://github.com/open-telemetry/opentelemetry-java/blob/master/QUICKSTART.md) documentation provides a lot of information on what the classes and their functions are used for. Since there is currently minimal documentation for C++, use the Python repository’s extensive documentation to learn more about how the API and SDK work. |
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.
Link is to Java but no mention of Java in the text?
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.
I believe those should be linking to the API and SDK documentation for Python, thanks for catching that. Will fix it
As Ankit mentioned in his PR, we moved build instructions for examples into this PR. I also moved the resources into CONTRIBUTING.md and fixed bad links. |
CONTRIBUTING.md
Outdated
* Take a look at this [Java SDK example.](https://github.com/open-telemetry/opentelemetry-java/tree/master/examples/sdk-usage) This shows a good use case of the SDK using stdout exporter. | ||
* Take a look at the [Java Jaeger example.](https://github.com/open-telemetry/opentelemetry-java/tree/master/examples/jaeger) This provides a brief introduction to the Jaeger exporter, its interface, and how to interact with the service. | ||
|
||
Please contribute! You’re welcome to add more information if you come across any helpful resources |
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.
nit: missing an end period.
CONTRIBUTING.md
Outdated
@@ -72,3 +72,60 @@ A PR is considered to be **ready to merge** when: | |||
* Urgent fixes can take exceptions as long as it has been actively communicated. | |||
|
|||
Any Approver / Maintainer can merge the PR once it is **ready to merge**. | |||
|
|||
## Build and Run Code Examples |
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.
I would suggest putting this section under Development
(above How to Send Pull Requests
).
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.
Nice improvements! LGTM assuming the comments are addressed.
|
||
### Code Examples | ||
|
||
* Follow the [simple trace example](https://github.com/open-telemetry/opentelemetry-cpp/pull/92) for an introduction to basic Open Telemetry functionality in C++. Currently the example can be found in [PR #94](https://github.com/open-telemetry/opentelemetry-cpp/pull/94) |
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.
When this is merged, can you submit an issue that reminds us of updating this document before we do a first release? Once we have our own examples and exporters in place, many of the examples here should be switched out.
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.
Yes, I'll get that issue up once this PR this merged
@pyohannes mentioned that he is okay to merge this PR during the community meeting. |
Rebased @reyang |
These are resources I found helpful as I was getting to learn about Open Telemetry and specifically the C++ repository. Adding these resources will help newcomers as they will have a central place to go to for resources. This Open Telemetry Onboarding Resources Guide was written by @HudsonHumphries, @ankit-bhargava and @Brandon-Kimberly