Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 readTheDocs #252
Add readTheDocs #252
Changes from 5 commits
877fca1
aaf0426
7ca3129
56c626a
a841407
85718be
5c2c132
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 have an OTel exporter in this repo that the doc build works correctly https://github.com/GoogleCloudPlatform/opentelemetry-operations-python/blob/c3772de4cf50950f1d51685ff3bbf0e46ee026df/docs/conf.py#L50-L56
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 this!
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.
So adding this
intersphinx-mapping
almost completely solved the problem! However there's still a few items that need to be ignore by thenitpick
...In nitpick-exceptions.ini I still have to silence the errors from these:
I added comments showing where the errors come from. This makes me wonder if I did the doc string for AWS components here wrong? Is there a certain way that I need to follow for the backticks `` when writing doc strings?
Something that would make the following doc string break such that I needed the
nitpick-exceptions
above?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 dont think the docstring should matter. Automodule should generate entries regardless of docstring, as long as you have the
.. automodule:: opentelemetry....
Do you see the AWS docs built locally?
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.
Just want to highlight this as my implementation of the solution described in the Stack Overflow post as mentioned in the PR description.
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.
Will this break the ref links? For example in the current stable django documentation https://opentelemetry-python.readthedocs.io/en/stable/instrumentation/django/django.html If you click on
Bases: [opentelemetry.instrumentation.instrumentor.BaseInstrumentor]
orSee [BaseInstrumentor]
They both take you BaseInstrumentor documentation. If I understand it correctly as these instrumentation libraries are moved out of main repo it can't find such references anymore and giving error. They are now cross-referencing targets from main repo docs.will
intersphinx_mapping
along with updating `BaseInstrumentor` occurrences with `opentelemetry.instrumentation.instrumentor.BaseInstrumentor` be helpful?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 believe you are correct that all the references in this
nitpick-exceptions.ini
file will produce links that are broken. It's because these components are Core code components and Contrib doesn't know where to link the reference to.The
intersphinx_mapping
solution sounds interesting! However it's not something I've explored 😓 although we can create a follow up to fix these links? If we want to keep this PR small we can merge it like this, and then we can create an issue to investigate that mapping solution to link to the Core docs?The main benefit this PR does is that at least we can have auto documentation on the packages even though the links to the Core components are broken.
Also, the docs will break if the SDK components break/change. But as I mentioned in the PR description, maybe that shouldn't be the job of the Docs, maybe that should be the job of the unit tests?
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.
This would be ideal. I just checked, all the references in this nitpick-exceptions.ini will not produce the broken links rather it is just a plain text instead of linked documentation. The
intersphinx_mapping
should really help us here for example pymongo instrumentation documentation has an link to official pymongo driver documentation hereThere 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.
Think you should be able to remove all of this with the intersphinx mapping?