-
Notifications
You must be signed in to change notification settings - Fork 501
[DOC] Document minimum required versions #3562
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
[DOC] Document minimum required versions #3562
Conversation
The libraries that opentelemetry-cpp requires have minimum required versions which are higher than the ones that are natively available on the systems being tested. By listing the exact versions of the libraries, the dependencies are clearly stated which removes the guesswork from build problems.
✅ Deploy Preview for opentelemetry-cpp-api-docs canceled.
|
Typo of authoritative.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3562 +/- ##
=======================================
Coverage 90.03% 90.03%
=======================================
Files 220 220
Lines 7069 7069
=======================================
Hits 6364 6364
Misses 705 705 🚀 New features to boost your workflow:
|
Thanks for the PR @markus456! The changes generally look good. Just a few CI failures on the |
The link used a relative path when the rest of them used absolue ones and the lines were above the 80 line limit.
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.
LGTM. Thanks for the PR!
It would’ve been sufficient to just reference |
That was what I initially was planning on doing but given that there are not that many external dependencies and that they appear to not change super frequently, I figured it'd be nicer for the end-user to just directly see it from the markdown document. If it's too much, it can of course be removed. |
This also helps ease of use, even if this means a (little) more maintenance on changes. Having to follow a trail of bread crumbs just to figure out what is needed to start with opentelemetry-cpp increases the barrier of entry, and the cost of taking the wrong path (build failures as reported) right from the start can be a show stopper preventing adoption. It is a tradeoff with maintenance, but has merits too. |
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.
LGTM, thanks for the doc.
What if we keep the single source but make the path more obvious? Like adding a clear callout in INSTALL.md, something like "Quick Start Prerequisites". Or maybe auto-generate this information from canonical source. Anyway, nothing against this PR - it's already merged and addresses a real user pain point. Just something to think about for future cases like this. For now, hopefully we can keep both places updated when versions change. |
The libraries that opentelemetry-cpp requires have minimum required versions which are higher than the ones that are natively available on the systems being tested. By listing the exact versions of the libraries, the dependencies are clearly stated which removes the guesswork from build problems.
This contribution is submitted by me as an individual.
Fixes #3560
Changes
Please provide a brief description of the changes here.
For significant contributions please make sure you have completed the following items:
CHANGELOG.md
updated for non-trivial changes