Skip to content

Remove references to ODBCVER and assume ODBC 3.x #19453

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

Merged
merged 6 commits into from
Aug 12, 2025

Conversation

NattyNarwhal
Copy link
Member

See the approved RFC. This also includes (and obsoletes) GH-17556, since we can safely assume that these functions are available in ODBC 3.x.

@NattyNarwhal
Copy link
Member Author

Just added UPGRADING comments and removed some useless defines on top of this, LMK if this still is good.

@TimWolla
Copy link
Member

Just added UPGRADING comments

This should also go into NEWS (for Beta 2). As for this change, I'm not qualified to meaningfully review this, but I'm not seeing any obvious issues. As per https://externals.io/message/128453, I'm requesting a RM review.

@TimWolla TimWolla requested a review from a team August 12, 2025 16:46
NattyNarwhal and others added 5 commits August 12, 2025 15:18
`SQLGetConnectOption`, `SQLSetConnectOption` and `SQLSetStmtOption` are
deprecated, so if ODBC 3 is available, we use `SQLSetConnectAttr`,
`SQLGetConnectAttr`, and `SQLSetStmtAttr` instead.

(This is based on phpGH-17556, but just assumes ODBC 3.x.)
We don't need to support the old way of doing it.
Again, no need for the version specific wrapper
Copy link
Member

@DanielEScherzer DanielEScherzer left a comment

Choose a reason for hiding this comment

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

looks primarily like removal of a bunch of stuff and replacement of some macros with their definitions, looks good to me wearing the RM hat

@NattyNarwhal NattyNarwhal merged commit 6004702 into php:master Aug 12, 2025
1 check passed
@DanielEScherzer
Copy link
Member

@NattyNarwhal the squashed commit message contains [skip ci] which is probably why CI was skipped for the overall commit in master

@NattyNarwhal
Copy link
Member Author

Oops, I thought that only applied to the first line of the commit. I'll take care to remove these from future squashed commit messages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants