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(instrumentation-tedious): support tedious@18 #2381

Merged
merged 3 commits into from
Aug 19, 2024

Conversation

trentm
Copy link
Contributor

@trentm trentm commented Aug 14, 2024

This builds on work by @kirrg001 in #2267.
tedious@18 added its own types, dropping the need to use
@types/tedious. However, it requires TypeScript v5 to use with TS,
which was challenging.

  • This uses test-all-versions' support for 'peerDependencies' to only
    install TS 5 when testing tedious@18.
  • This drops ts-mocha, in favour of 'mocha --require ts-node/register'
    as is being used in the core repo.
  • This adds skipLibCheck:true to the tsconfig, needed to compile
    the tests when using tedious@18. It was that or bump to
    target:es2022, which could impact the published code. JS SIG
    discussion decided using skipLibCheck is probably fine.

More discussion here: #2267 (comment)

Obsoletes: #2267
Closes: #2266

tedious@18 adds its own .d.ts files. Compiling with them requires
TypeScript v5, and either target:'es2022' for 'AggregateError'
or using skipLibCheck:true in tsconfig.json.

This change limits the update to TypeScript v5 to just while
testing tedious@18 via test-all-versions' support for
'peerDependencies'.

THis also drops ts-mocha in favour of 'mocha --require ts-node/register'
as was done on opentelemetry-js.git in open-telemetry/opentelemetry-js#4840

Closes: open-telemetry#2266
@trentm trentm self-assigned this Aug 14, 2024
@trentm trentm requested a review from a team August 14, 2024 21:51
@github-actions github-actions bot added pkg:instrumentation-tedious pkg-status:unmaintained This package is unmaintained. Only bugfixes may be acceped until a new owner has been found. labels Aug 14, 2024
@trentm trentm requested a review from kirrg001 August 14, 2024 21:51
Copy link

codecov bot commented Aug 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.56%. Comparing base (dfb2dff) to head (2dc69ce).
Report is 212 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2381      +/-   ##
==========================================
- Coverage   90.97%   90.56%   -0.42%     
==========================================
  Files         146      157      +11     
  Lines        7492     7631     +139     
  Branches     1502     1574      +72     
==========================================
+ Hits         6816     6911      +95     
- Misses        676      720      +44     
Files Coverage Δ
...ode/instrumentation-tedious/src/instrumentation.ts 92.30% <ø> (-0.17%) ⬇️

... and 74 files with indirect coverage changes

@trentm
Copy link
Contributor Author

trentm commented Aug 14, 2024

For the record: some earlier skipLibCheck discussion here: #2165

Copy link
Contributor

This package does not have an assigned component owner and is considered unmaintained. As such this package is in feature-freeze and this PR will be closed with 14 days unless a new owner or a sponsor (a member of @open-telemetry/javascript-approvers) for the feature is found. It is the responsibility of the author to find a sponsor for this feature.
Are you familiar with this package? Consider becoming a component owner.

Copy link
Contributor

@kirrg001 kirrg001 left a comment

Choose a reason for hiding this comment

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

LGTM

@trentm trentm added the has:sponsor This package or feature has a sponsor that has volunteered to review PRs and respond to questions label Aug 15, 2024
@trentm trentm merged commit 0e9791a into open-telemetry:main Aug 19, 2024
21 checks passed
@trentm trentm deleted the tm-tedious-18-play branch August 19, 2024 22:42
@dyladan dyladan mentioned this pull request Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has:sponsor This package or feature has a sponsor that has volunteered to review PRs and respond to questions pkg:instrumentation-tedious pkg-status:unmaintained:autoclose-scheduled pkg-status:unmaintained This package is unmaintained. Only bugfixes may be acceped until a new owner has been found.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for tedious v18
3 participants