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

chore(tedious): support new tedious versions 15, 16 #1638

Closed
wants to merge 7 commits into from

Conversation

blumamir
Copy link
Member

@blumamir blumamir commented Aug 15, 2023

replaces: #1521

When the tedious instrumentations was written, the latest version was 14.
Since then, tedious package released versions 15 and 16, for which renovate opened to following PRs: #1403 #1521 .
So it turns out that tedious is already compatible and patches new versions. This PR is only about aligning the supported versions and update the tests to use new tedious versions.

Prior to this PR, the instrumentation was patching version range *, which isn't very safe as new major versions can introduce breaking changes which the instrumentation might not be ready for.

This PR replaces the unmergable renovate PR, by introducing the following changes:

  1. Cap the maximum supported version to latest (16) instead of *. This algins the instrumentation to the practice which is common in this repo and safer.
  2. Update the README to advertise the supported versions range accordingly
  3. Add the new versions to TAV.
  4. Do not run tedious tav for node 14, as the new versions dropped support for it which obviously fails to install in node 14.
  5. Upgrade the tedious version for npm run test to use the most downloaded version which is not 16 (which dropped support for node 14) - 15.1.3, and the "@types/tedious" to latest - ^4.0.9

As I am using a mac with apple chip, for which the mcr.microsoft.com/mssql/server:2017-latest docker image has no compatible distribution, I am not able to run the unit-tests locally, so uploading it here to test on CI. If you are aware of a better workflow, please let me know.

@blumamir blumamir requested a review from a team as a code owner August 15, 2023 15:46
@blumamir blumamir marked this pull request as draft August 15, 2023 15:46
@github-actions github-actions bot requested a review from rauno56 August 15, 2023 15:46
@codecov
Copy link

codecov bot commented Aug 15, 2023

Codecov Report

Merging #1638 (740a7be) into main (a556179) will decrease coverage by 0.37%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1638      +/-   ##
==========================================
- Coverage   91.77%   91.40%   -0.37%     
==========================================
  Files         139       49      -90     
  Lines        7112     2432    -4680     
  Branches     1427      442     -985     
==========================================
- Hits         6527     2223    -4304     
+ Misses        585      209     -376     
Files Changed Coverage Δ
...ode/instrumentation-tedious/src/instrumentation.ts 10.75% <ø> (-81.73%) ⬇️

... and 91 files with indirect coverage changes

@blumamir blumamir changed the title chore: support new tedious versions chore(tedious): support new tedious versions 15, 16 Aug 15, 2023
@blumamir
Copy link
Member Author

I wasn't able to make tests green for tedious >=16 and node 14.

Documented the task in #1656 and fixed the repo code to cap support at the last testable version which is 15.

@blumamir blumamir closed this Aug 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants