-
Notifications
You must be signed in to change notification settings - Fork 534
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
feat: add supported node versions for all packages #973
Conversation
Codecov Report
@@ Coverage Diff @@
## main #973 +/- ##
==========================================
- Coverage 95.91% 92.15% -3.76%
==========================================
Files 13 129 +116
Lines 856 5925 +5069
Branches 178 1144 +966
==========================================
+ Hits 821 5460 +4639
- Misses 35 465 +430
|
strictly speaking only modules using the performance times require 8.12 others work fine with lower versions. But not sure if there is any benefit of having e.g. a resource detector working with 8.0 but there is no SDK to use it with. |
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.
This will break users if they are running on node >=8.5.0 && <=8.12.0
Not sure if there are any of these out there 🤷♂️
Could be nice to at least bump the minor version for publishing this change (e.g. label it as "feat"?)
That's right. In that sense we don't absolutely need to narrow down propagators and detectors. I think I'd approached it differently if we had SDK compatible with 8.5 and took it from module<>plugin direction making the version minimum of what either can support. Would you prefer it solved differently? |
I think it's fine. I doubt that it has much impact at all as people using that old node versions are most likely used to ignore warnings anyway :o). |
Which problem is this PR solving?
In accordance with open-telemetry/opentelemetry-js#2834 we've changed the support range to
>=8.12
.open-telemetry/opentelemetry-js@da22673 changed that in Core.
Short description of the changes
Updated every
package.json
according to the issue above and out CI configuration, which skips tests for some of the packaged that have dropped the support for older node versions.