Skip to content

feat: Adds minimal support for Devart Oracle client. #1580

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

Merged
merged 1 commit into from
May 17, 2023

Conversation

PhenX
Copy link
Contributor

@PhenX PhenX commented Apr 27, 2023

BEGIN_COMMIT_OVERRIDE
feat: Adds minimal support for Devart Oracle client.
END_COMMIT_OVERRIDE

Description

This adds support for auto instrumentation for the Devart Oracle driver

Author Checklist

  • Unit tests, Integration tests, and Unbounded tests completed
  • Performance testing completed with satisfactory results (if required)

Reviewer Checklist

  • Perform code review
  • Pull request was adequately tested (new/existing tests, performance tests)

@CLAassistant
Copy link

CLAassistant commented Apr 27, 2023

CLA assistant check
All committers have signed the CLA.

@chynesNR
Copy link
Member

Hi @PhenX, thanks so much for your submission! Typically when we add new instrumentation, we add integration tests for it. Would you be comfortable taking that on, or would you like us to do it? We'd be happy to, but it may take a few days for us to get to it.

@PhenX
Copy link
Contributor Author

PhenX commented Apr 28, 2023

@chynesNR thank you for your quick answer, I'll give it a try, and take inspiration from the other SQL instrumentations. I'll tell you if it is too much for me.

@PhenX
Copy link
Contributor Author

PhenX commented Apr 28, 2023

I can't find integration tests for Oracle, I think I won't be able to start from scratch, feel free to handle it :)

@chynesNR
Copy link
Member

chynesNR commented May 1, 2023

@PhenX - No worries! I should have time to get to it this week.

@PhenX
Copy link
Contributor Author

PhenX commented May 11, 2023

Hello @chynesNR any news on this ? :)

@chynesNR
Copy link
Member

Hi @PhenX - so sorry for the delay! Our goal is to get it done this week, but if not, we'll merge this and add a ticket to the backlog for the integration tests. I think this is a low-risk change.

Copy link
Member

@nr-ahemsath nr-ahemsath left a comment

Choose a reason for hiding this comment

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

Low risk change. Going to merge without integration tests and add a backlog item to add them at a later date.

@tippmar-nr tippmar-nr changed the title Add minimal support for Devart Oracle client feat: Adds minimal support for Devart Oracle client. May 16, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #1580 (82c902c) into main (d2b4b0d) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1580      +/-   ##
==========================================
+ Coverage   74.85%   74.87%   +0.01%     
==========================================
  Files         408      408              
  Lines       25602    25602              
==========================================
+ Hits        19164    19169       +5     
+ Misses       6438     6433       -5     

see 1 file with indirect coverage changes

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.

6 participants