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

Remove Web server prerequisite #839

Conversation

Majkl578
Copy link

@Majkl578 Majkl578 commented Sep 3, 2018

Web server is not required to use the MSSQL Driver and it's definitely a prerequisite.


This change is Reviewable

@coveralls
Copy link

Coverage Status

Coverage remained the same at 75.148% when pulling b2b4264 on Majkl578:readme-remove-webserver-prerequisite into 9654020 on Microsoft:master.

@codecov-io
Copy link

codecov-io commented Sep 3, 2018

Codecov Report

Merging #839 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #839   +/-   ##
=======================================
  Coverage   80.06%   80.06%           
=======================================
  Files          25       25           
  Lines        7325     7325           
=======================================
  Hits         5865     5865           
  Misses       1460     1460
Impacted Files Coverage Δ
...-7.1.20-src/ext/pdo_sqlsrv/shared/core_results.cpp
...phpdev/vc14/x86/php-7.1.20-src/ext/sqlsrv/stmt.cpp
...php-7.1.20-src/ext/pdo_sqlsrv/shared/core_stmt.cpp
...x86/php-7.1.20-src/ext/sqlsrv/shared/core_init.cpp
...phpdev/vc14/x86/php-7.1.20-src/ext/sqlsrv/conn.cpp
...php-7.1.20-src/ext/pdo_sqlsrv/shared/core_conn.cpp
...phpdev/vc14/x86/php-7.1.20-src/ext/sqlsrv/init.cpp
...c14/x86/php-7.1.20-src/ext/pdo_sqlsrv/pdo_util.cpp
...ev/vc14/x86/php-7.1.20-src/ext/sqlsrv/php_sqlsrv.h
.../php-7.1.20-src/ext/sqlsrv/shared/core_results.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 9654020...b2b4264. Read the comment docs.

@yitam
Copy link
Contributor

yitam commented Sep 4, 2018

hi @Majkl578 , while it is true a web server is not absolutely needed, note that most PHP users configure IIS / Apache.

@Majkl578
Copy link
Author

Majkl578 commented Sep 4, 2018

Or FPM + nginx.
But that doesn't make it a prerequisite for installing this extension. There is barely any relation between web server SAPI and using mssql extension, also you don't need a webserver configured to run PHP to use it. Right now this feels like a forced IIS advertisement.

@yitam
Copy link
Contributor

yitam commented Sep 4, 2018

You're right @Majkl578 . We will start updating our readme and/or other online docs for the upcoming preview, and we will consider your suggestion.

@Majkl578
Copy link
Author

Majkl578 commented Sep 4, 2018

Thanks. 👍

@yitam
Copy link
Contributor

yitam commented Sep 4, 2018

Closing this pull request, @Majkl578
See related #841

@yitam yitam closed this Sep 4, 2018
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