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

fix(pino): removed the tav for versions ^8 #1146

Merged
merged 2 commits into from
Sep 1, 2022

Conversation

osherv
Copy link
Member

@osherv osherv commented Sep 1, 2022

Which problem is this PR solving?

Pinto test-all-versions for versions <8 fail the test-all versions.
Also, according to the documentation, the instrumentation does not support versions that are greater than ^7.11.

Short description of the changes

Remove the testing for version <8 and open a new issue to support versions <8

The failed Ci:
https://github.com/open-telemetry/opentelemetry-js-contrib/runs/8117913889?check_suite_focus=true

Checklist

  • Ran npm run test-all-versions for the edited package(s) on the latest commit if applicable.

@osherv osherv requested a review from a team September 1, 2022 13:15
@osherv
Copy link
Member Author

osherv commented Sep 1, 2022

The following issue: #1147

@codecov
Copy link

codecov bot commented Sep 1, 2022

Codecov Report

Merging #1146 (6735d9f) into main (4fb1a5a) will increase coverage by 0.33%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1146      +/-   ##
==========================================
+ Coverage   96.07%   96.41%   +0.33%     
==========================================
  Files          14       16       +2     
  Lines         892     1004     +112     
  Branches      191      208      +17     
==========================================
+ Hits          857      968     +111     
- Misses         35       36       +1     
Impacted Files Coverage Δ
...tapackages/auto-instrumentations-node/src/utils.ts 98.00% <0.00%> (ø)
...emetry-instrumentation-pino/src/instrumentation.ts 100.00% <0.00%> (ø)

@dyladan
Copy link
Member

dyladan commented Sep 1, 2022

Looks like it was an accidental addition when @rauno56 cleaned up the tav version? #1064

@osherv
Copy link
Member Author

osherv commented Sep 1, 2022

@dyladan Thanks for the reference, that is exactly what failed the tav.
Probably added by mistake

@dyladan
Copy link
Member

dyladan commented Sep 1, 2022

branch out of date. please allow maintainers to edit your branch or update the branch or it cannot be merged

@dyladan dyladan merged commit 078ab2d into open-telemetry:main Sep 1, 2022
@dyladan dyladan mentioned this pull request Sep 1, 2022
@osherv osherv deleted the fix/remove-pino-v8-support branch September 6, 2022 12:22
@francisdb francisdb mentioned this pull request Sep 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants