docs(opentelemetry-browser-detector): Use tree-shakeable string constants for semconv in Readme #4768
Conversation
There was a problem hiding this comment.
Since this is only a change in the README, I think we are okay to describe this PR as "docs" instead of "refactor".
Also it looks like "@opentelemetry/semantic-conventions": "1.25.0" is in the package.json but I don't see it being used anywhere. I think we could uninstall it from this package (in this package directory run npm uninstall @opentelemetry/semantic-conventions). We will also find out if I'm missing it somewhere and this causes a test to fail.
JohannesHuster
force-pushed
the
browser-detector-semconv
branch
from
196b1b2 to
9c3da62
Compare
JohannesHuster
changed the title
|
@JamieDanielson Sounds good. Thanks for the pointers :-) I pushed the changes and renamed commit, PR and changelog entry. |
experimental/CHANGELOG.md
Outdated
| @@ -61,6 +61,7 @@ All notable changes to experimental packages in this project will be documented | |||
| * docs(instrumentation): better docs for supportedVersions option [#4693](https://github.com/open-telemetry/opentelemetry-js/pull/4693) @blumamir | |||
| * docs: align all supported versions to a common format [#4696](https://github.com/open-telemetry/opentelemetry-js/pull/4696) @blumamir | |||
| * refactor(examples): use new exported string constants for semconv in experimental/examples/opencensus-shim [#4763](https://github.com/open-telemetry/opentelemetry-js/pull/4763#pull) @Zen-cronic | |||
| * docs(opentelemetry-browser-detector): Use tree-shakeable string constants for semconv in Readme [#4768](https://github.com/open-telemetry/opentelemetry-js/pull/4768) @JohannesHuster | |||
There was a problem hiding this comment.
Sorry for the churn, this will need to move up into the Unreleased section now
There was a problem hiding this comment.
@JamieDanielson No worries, always good to see a release! I added the change as a new commit for now, to make reviewing easier, but I'm happy to rebase and remove some unnecessary commits before merging.
There was a problem hiding this comment.
Failing tests are already being addressed here, I think: #4777
There was a problem hiding this comment.
The test fix was merged so I've merged main here. Hopefully that passes now 🤞
There was a problem hiding this comment.
Also just saw your other comment, no rebase is necessary 🙂 . Rebases can make it difficult to see what changes have occurred during review.
There was a problem hiding this comment.
Perfect, thanks 🙂 Tests are looking good now 😌
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4768 +/- ##
==========================================
+ Coverage 91.04% 91.11% +0.06%
==========================================
Files 89 140 +51
Lines 1954 3027 +1073
Branches 416 677 +261
==========================================
+ Hits 1779 2758 +979
- Misses 175 269 +94 |
|
^^^ manual merge from main |
|
I'll give it a day in case @JamieDanielson wants to add anything else, then I'll merge this. |
539d942
…ants for semconv in Readme (open-telemetry#4768) * docs(opentelemetry-browser-detector): Use tree-shakeable string constants for semconv in Readme * Update changelog * Uninstall unused semantic-conventions package from browser-detector package * Move changelog entry to Unreleased section * rm changelog entry, I don't think it is necessary for a small docs update --------- Co-authored-by: Jamie Danielson <[email protected]> Co-authored-by: Trent Mick <[email protected]>
Which problem is this PR solving?
Updates #4567
Short description of the changes
Replace deprecated imports from
@opentelemetry/semantic-conventionswith new (tree-shakeable) string constants for package@opentelemetry/opentelemetry-browser-detector. What I did:Type of change
How Has This Been Tested?
Checklist: