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

Set logger for driver API #1107

Merged
merged 10 commits into from
Mar 25, 2020
Merged

Set logger for driver API #1107

merged 10 commits into from
Mar 25, 2020

Conversation

yitam
Copy link
Contributor

@yitam yitam commented Mar 24, 2020

No description provided.

@coveralls
Copy link

coveralls commented Mar 24, 2020

Coverage Status

Coverage decreased (-0.07%) to 74.401% when pulling 8733377 on yitam:prototype2 into fb335c0 on microsoft:dev.

@codecov
Copy link

codecov bot commented Mar 24, 2020

Codecov Report

Merging #1107 into dev will decrease coverage by 0.19%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #1107      +/-   ##
==========================================
- Coverage   76.92%   76.72%   -0.20%     
==========================================
  Files          25       25              
  Lines        7424     7417       -7     
==========================================
- Hits         5711     5691      -20     
- Misses       1713     1726      +13     
Impacted Files Coverage Δ
.../phpdev/vc15/x86/php-7.4.3-src/ext/sqlsrv/init.cpp
.../vc15/x86/php-7.4.3-src/ext/pdo_sqlsrv/pdo_dbh.cpp
.../phpdev/vc15/x86/php-7.4.3-src/ext/sqlsrv/conn.cpp
.../x86/php-7.4.3-src/ext/sqlsrv/shared/core_util.cpp
...86/php-7.4.3-src/ext/sqlsrv/shared/core_stream.cpp
.../php-7.4.3-src/ext/pdo_sqlsrv/php_pdo_sqlsrv_int.h
.../php-7.4.3-src/ext/pdo_sqlsrv/shared/core_init.cpp
...15/x86/php-7.4.3-src/ext/pdo_sqlsrv/pdo_parser.cpp
.../php-7.4.3-src/ext/pdo_sqlsrv/shared/core_conn.cpp
...vc15/x86/php-7.4.3-src/ext/pdo_sqlsrv/pdo_stmt.cpp
... and 40 more

@yitam yitam requested review from v-mabarw and David-Engel March 24, 2020 17:59
v-mabarw
v-mabarw previously approved these changes Mar 24, 2020
Copy link
Contributor

@v-mabarw v-mabarw left a comment

Choose a reason for hiding this comment

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

Looks good, just noticed a few minor things.

@@ -0,0 +1,56 @@
--TEST--
Tset functions return FALSE for errors with logging
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Tset functions return FALSE for errors with logging
Test functions return FALSE for errors with logging

@@ -0,0 +1,49 @@
--TEST--
Tset functions return FALSE for errors with logging
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Tset functions return FALSE for errors with logging
Test functions return FALSE for errors with logging


// get the structure
ss_sqlsrv_conn *conn = static_cast<ss_sqlsrv_conn*>( rsrc->ptr );
SQLSRV_ASSERT( conn != NULL, "sqlsrv_conn_dtor: connection was null");

SET_FUNCTION_NAME( *conn );
conn->set_func("sqlsrv_conn_dtor");
Copy link
Contributor

Choose a reason for hiding this comment

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

Would _FN_ work here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not this one because the macro LOG_FUNCTION is commented but I can certainly use __func__
Thanks :)

@yitam yitam merged commit 7214e8d into microsoft:dev Mar 25, 2020
@yitam yitam deleted the prototype2 branch May 5, 2020 01:12
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