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(plugin-document-load): check if getEntriesByType is available before using it #259

Merged
merged 3 commits into from Dec 10, 2020
Merged

fix(plugin-document-load): check if getEntriesByType is available before using it #259

merged 3 commits into from Dec 10, 2020

Conversation

mhennoch
Copy link
Contributor

@mhennoch mhennoch commented Nov 9, 2020

Which problem is this PR solving?

  • When performance.getEntriesByType is undefined error is thrown(and fallback performance.timing is not used). This happens in Safari 10.1 for example where getEntriesByType is undefined but performance.timing is available.

Short description of the changes

  • Check if performance.getEntriesByType is present before using it.

getEntriesByType is available before using it
@mhennoch mhennoch requested a review from a team as a code owner November 9, 2020 10:50
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Nov 9, 2020

CLA Signed

The committers are authorized under a signed CLA.

@codecov
Copy link

codecov bot commented Nov 9, 2020

Codecov Report

Merging #259 (6cae7c6) into master (d896904) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #259   +/-   ##
=======================================
  Coverage   95.07%   95.07%           
=======================================
  Files         110      110           
  Lines        5888     5888           
  Branches      607      609    +2     
=======================================
  Hits         5598     5598           
  Misses        290      290           
Impacted Files Coverage Δ
...opentelemetry-instrumentation-graphql/src/types.ts 100.00% <ø> (ø)

Copy link
Member

@obecny obecny left a comment

Choose a reason for hiding this comment

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

lgtm, thx for contribution

@mhennoch
Copy link
Contributor Author

Any idea when this will get merged?

@obecny
Copy link
Member

obecny commented Dec 8, 2020

@mhennoch we cannot merge until you sign the CLA

@mhennoch
Copy link
Contributor Author

mhennoch commented Dec 8, 2020

@obecny I signed it after it initially failed. Under checks it says:
EasyCLA — EasyCLA check passed. You are authorized to contribute.

@obecny
Copy link
Member

obecny commented Dec 8, 2020

it was marked as not signed, until you commented on PR then the CLA check is run again, now I see it is fine

@obecny obecny merged commit 4ed223b into open-telemetry:master Dec 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants