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.

Sign up for GitHub

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

Jump to bottom

Changelog updated #816

Merged
merged 6 commits into from
Jul 19, 2018
Merged

Changelog updated #816

merged 6 commits into from
Jul 19, 2018

Conversation

david-puglielli
Copy link
Contributor

@david-puglielli david-puglielli commented Jul 18, 2018 •
edited by lilgreenbird
Loading

This change is Reviewable

@david-puglielli david-puglielli requested a review from yitam July 18, 2018 23:46
yitam
yitam reviewed Jul 18, 2018
View reviewed changes
Copy link
Contributor

@yitam yitam left a comment

Choose a reason for hiding this comment

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

Please put all the links back for references

CHANGELOG.md Outdated
- In Linux and macOS, setlocale() only takes effect if it is invoked before the first connection. Attempting to set the locale after connecting will not work
- Always Encrypted requires [MS ODBC Driver 17+](https://docs.microsoft.com/en-us/sql/connect/odbc/linux-mac/installing-the-microsoft-odbc-driver-for-sql-server?view=sql-server-2017)
- Only Windows Certificate Store and Azure Key Vault are supported. Custom Keystores are not supported
- Issue #716 - With Always Encrypted enabled, named parameters in subqueries are not supported
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion: Custom Keystore provider is not yet supported

@david-puglielli
Copy link
Contributor Author

The full links are not needed in github. Do we still need them in the markdown source?

@yitam
Copy link
Contributor

yitam commented Jul 18, 2018

@david-puglielli I see your point. Then never mind. Thanks!

@coveralls
Copy link

coveralls commented Jul 18, 2018 •
edited
Loading

Coverage Status

Coverage increased (+0.2%) to 75.193% when pulling 0f66c48 on david-puglielli:changelog-update-5.3.0 into 35631cf on Microsoft:dev.

@codecov-io
Copy link

codecov-io commented Jul 19, 2018 •
edited
Loading

Codecov Report

Merging #816 into dev will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##              dev     #816   +/-   ##
=======================================
  Coverage   80.06%   80.06%           
=======================================
  Files          25       25           
  Lines        7325     7325           
=======================================
  Hits         5865     5865           
  Misses       1460     1460
Impacted Files Coverage Δ
...-7.1.19-src/ext/pdo_sqlsrv/shared/core_results.cpp
...6/php-7.1.19-src/ext/sqlsrv/shared/core_stream.cpp
...4/x86/php-7.1.19-src/ext/pdo_sqlsrv/pdo_parser.cpp
...p-7.1.19-src/ext/pdo_sqlsrv/shared/core_stream.cpp
...phpdev/vc14/x86/php-7.1.19-src/ext/sqlsrv/conn.cpp
...php-7.1.19-src/ext/pdo_sqlsrv/shared/core_stmt.cpp
...phpdev/vc14/x86/php-7.1.19-src/ext/sqlsrv/util.cpp
...ev/vc14/x86/php-7.1.19-src/ext/sqlsrv/php_sqlsrv.h
...vc14/x86/php-7.1.19-src/ext/pdo_sqlsrv/pdo_dbh.cpp
...c14/x86/php-7.1.19-src/ext/pdo_sqlsrv/pdo_stmt.cpp
... and 40 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 35631cf...0f66c48. Read the comment docs.

yitam
yitam reviewed Jul 19, 2018
View reviewed changes
test/functional/sqlsrv/skipif_version_less_than_2k14.inc
@@ -23,7 +23,7 @@ $msodbcsql_maj = explode(".", $msodbcsql_ver)[0];
$msodbcsql_min = explode(".", $msodbcsql_ver)[1];

if (!$is_win) {
if ($msodbcsql_maj < 17 or $msodbcslq_min < 2) {
if ($msodbcsql_maj < 17 or $msodbcsql_min < 2) {
die("skip Unsupported ODBC driver version");
Copy link
Contributor

Choose a reason for hiding this comment

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

The same typo is in pdo_sqlsrv too

yitam
yitam suggested changes Jul 19, 2018
View reviewed changes
Copy link
Contributor

@yitam yitam left a comment

Choose a reason for hiding this comment

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

Please fix the errors in conn res tests.

yitam
yitam approved these changes Jul 19, 2018
View reviewed changes
Copy link
Contributor

@yitam yitam left a comment

Choose a reason for hiding this comment

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

Finally!

@david-puglielli david-puglielli merged commit faf5ce8 into microsoft:dev Jul 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yitam yitam yitam approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@david-puglielli @yitam @coveralls @codecov-io