Skip to content
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: Added pkgVersion to shim instances to facilitate semver checking without having to re-parse the package.json #1883

Merged
merged 1 commit into from Nov 21, 2023

Conversation

bizob2828
Copy link
Member

Description

In #1799 we added parsing package.json to all packages getting instrumented in shimmer. There were a few places where it was later parsing package.json. This PR assigns the pkgVersion to the shim instance so it can be read in instrumentation instead of re-parsing package.json. The longer term goal will be to build semver checking into instrumentation registration but that's for another PR. This also reduces re-parsing package.json to track instrumentation usage as it's on the shim instance.

Related Issues

This actually closes #1860 because it does not try to parse package.json any longer.

@jsumners-nr jsumners-nr self-assigned this Nov 21, 2023
…g without having to re-parse the package.json
Copy link
Contributor

@jsumners-nr jsumners-nr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

Copy link

codecov bot commented Nov 21, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (86d83f2) 96.87% compared to head (3afd5ad) 96.87%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1883      +/-   ##
==========================================
- Coverage   96.87%   96.87%   -0.01%     
==========================================
  Files         209      209              
  Lines       39635    39699      +64     
==========================================
+ Hits        38397    38458      +61     
- Misses       1238     1241       +3     
Flag Coverage Δ
integration-tests-16.x 78.76% <89.47%> (+0.02%) ⬆️
integration-tests-18.x 79.02% <89.47%> (-0.01%) ⬇️
integration-tests-20.x 79.03% <89.47%> (+<0.01%) ⬆️
unit-tests-16.x 91.09% <95.08%> (-0.09%) ⬇️
unit-tests-18.x 91.07% <95.08%> (-0.09%) ⬇️
unit-tests-20.x 91.07% <95.08%> (-0.09%) ⬇️
versioned-tests-16.x 73.68% <98.43%> (+0.08%) ⬆️
versioned-tests-18.x 73.68% <98.43%> (+0.08%) ⬆️
versioned-tests-20.x 73.69% <98.43%> (+0.08%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bizob2828 bizob2828 merged commit 4ddfd81 into newrelic:main Nov 21, 2023
23 of 24 checks passed
Node.js Engineering Board automation moved this from Needs PR Review to Done: Issues recently completed Nov 21, 2023
@github-actions github-actions bot mentioned this pull request Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Node.js Engineering Board
  
Done: Issues recently completed
Development

Successfully merging this pull request may close these issues.

Core module domain registered as third-party
2 participants