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

Simplied conversions from strings to numbers #1146

Merged
merged 5 commits into from
Jun 26, 2020
Merged

Simplied conversions from strings to numbers #1146

merged 5 commits into from
Jun 26, 2020

Conversation

yitam
Copy link
Contributor

@yitam yitam commented Jun 23, 2020

When using client buffers, fetching strings as doubles or integers is inconsistent across platforms. Simplified the corresponding helper methods. Also, no need to keep the numbers to wide strings functions because the drivers no longer convert numeric values to wide strings.

@coveralls
Copy link

coveralls commented Jun 24, 2020

Coverage Status

Coverage increased (+0.3%) to 75.376% when pulling ad8a1c7 on yitam:strtonum into d5e1d8c on microsoft:dev.

@codecov
Copy link

codecov bot commented Jun 24, 2020

Codecov Report

Merging #1146 into dev will increase coverage by 0.89%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #1146      +/-   ##
==========================================
+ Coverage   77.87%   78.76%   +0.89%     
==========================================
  Files          24       24              
  Lines        7280     7262      -18     
==========================================
+ Hits         5669     5720      +51     
+ Misses       1611     1542      -69     
Impacted Files Coverage Δ
...vc15/x86/php-7.4.7-src/ext/pdo_sqlsrv/pdo_stmt.cpp 81.75% <0.00%> (+0.47%) ⬆️
...p-7.4.7-src/ext/pdo_sqlsrv/shared/core_results.cpp 58.31% <0.00%> (+4.97%) ⬆️
...6/php-7.4.7-src/ext/sqlsrv/shared/core_results.cpp 72.54% <0.00%> (+6.99%) ⬆️

@yitam yitam requested a review from v-mabarw June 24, 2020 19:21
Copy link
Member

@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.

Other than some remaining cleanup, looks good to me.

@yitam yitam merged commit ecbd53f into microsoft:dev Jun 26, 2020
@yitam yitam deleted the strtonum branch June 26, 2020 22:56
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.

3 participants