-
Notifications
You must be signed in to change notification settings - Fork 518
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: bumped aws components to 1.0 #658
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.
🎉
Codecov Report
@@ Coverage Diff @@
## main #658 +/- ##
==========================================
+ Coverage 96.87% 97.77% +0.89%
==========================================
Files 11 29 +18
Lines 640 1436 +796
Branches 126 195 +69
==========================================
+ Hits 620 1404 +784
- Misses 20 32 +12
|
This is in conflict with the current lerna.json which claims all packages to be the same version. I think this also has to wait until the release automation is updated to allow for releasing packages with independent versions. The problems we had with release-please in core generally don't apply to this repo since modules do not depend on each other so it should be possible to use release-please here without issue. If you would like to take a crack at porting the PR I did in core to this repo it would be quite helpful. |
Makes sense, I will leave it in draft until the release-please automation is in place and close if it's no longer necessary. I wasn't sure if release-please would be capable of automatically bumping to 1.0.0, since it's at 0.24.0 right now and even if we tagged a change as breaking, I don't know if that would bump the major or minor version.
Sure, we're talking about open-telemetry/opentelemetry-js#2431 just to confirm right? I'll see if I have the bandwidth. |
I guess this answers my question: https://github.com/open-telemetry/opentelemetry-js/blob/5fefbb274893a93033ef3786f75173c62682ade7/release-please-config.json#L4 So I think this PR will still be needed even after adding release-please. |
Opened #659 to add release-please to this repo. |
Release-As: 1.0.0
Release-As: 1.0.0
Release-As: 1.0.0
@dyladan PTAL I reverted the manual Not sure if this will work since we're targeting several packages unlike the docs, but I think as long as we maintain the |
Is it intended that v1.0 of these packages depends on 0.25.0 of core instead 1.0.0? |
@Flarna good catch thank you, updated to SDK 1.0.0 |
@dyladan any chance this could be merged to include in the next release? |
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 don't see the initial changes that bumped the package, API and SDK versions anymore - updated in upstream?
@@ -38,16 +38,15 @@ const tracerProvider = new NodeTracerProvider(tracerConfig); | |||
|
|||
Example trace ID format: 58406520a006649127e371903a2de979 | |||
|
|||
A trace ID consists of two parts; the timestamp and the unique identifier. | |||
A trace ID consists of two parts: the timestamp and the unique identifier. | |||
|
|||
#### Time Stamp |
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 know changing the docs wasn't the intention of the PR, but it caught my eye since you did changes in the readme file as well. 😆
#### Time Stamp | |
#### Timestamp |
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.
Since you were at it, I'd actually just format the whole section to be more concise:
A trace ID consists of two parts:
- Timestamp: The first 8 hexadecimal digits represent the time of the original request in Unix epoch time. For example, 10:00 AM December 1st, 2016 PST in epoch time is 1480615200 seconds, or 58406520 in hexadecimal digits.
- Unique Identifier: The last 24 hexadecimal digits is an random identifier for the trace.
But that comes to style more than correctness.
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 the guidance, I updated it as such! I was not a fan of the old formatting either.
@dyladan Can this be merged? |
👋 Howdy! Any idea of when we can expect a new release of the packages touched in this PR? My organization is sitting on a PR to update to the We haven't actually observed any strange behavior when packages use different versions of the @otel/core lib, but it sounds like the risk for unexpected behavior here is high: |
Short description of the changes