Skip to content

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 David-Engel and v-mabarw 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

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