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

Add Sqlsrv driver #3714

Merged
merged 59 commits into from
Oct 19, 2020
Merged

Add Sqlsrv driver #3714

merged 59 commits into from
Oct 19, 2020

Conversation

michalsn
Copy link
Member

@michalsn michalsn commented Oct 2, 2020

Description
This PR adds Sqlsrv driver based on @qury work. I'm not experienced with MSSQL but not many changes were needed.

The code has been cleaned up a bit and a few minor changes have been made to make the tests working.

Some tweaks are probably needed, so if anyone takes the time to review it, I would appreciate adding code suggestions to make things easier for me.

Closes #3612

Checklist:

  • Securely signed commits
  • Unit testing, with >80% coverage
  • Conforms to style guide

@lonnieezell
Copy link
Member

You rock! Thanks for getting this one across the finish line, and @qury - you're a rockstar! Thanks for all of your work on this.

@michalsn
Copy link
Member Author

Seems like we have a temporary problem with sqlsrv extension (not available) after updating from PHP 7.4.10 to PHP 7.4.11. I tried to use a specific version number (the latest preview) as a workaround but without a luck 😕

@samsonasik
Copy link
Member

is it possible to pin php-version to 7.4.10 ?

@michalsn
Copy link
Member Author

No, we can only specify a minor version 😞

@michalsn
Copy link
Member Author

Well... apparently on Monday everything decided to start working again 🤷‍♂️

@michalsn
Copy link
Member Author

@lonnieezell @MGatner @samsonasik @paulbalandan @qury All tests look good. I think I would like to merge it at this stage. This will allow other developers to battle-test it even more and then we should be able to fix any possible bugs before the next release.

@MGatner
Copy link
Member

MGatner commented Oct 19, 2020

I agree. I still haven't actually used it but I've been through the code and read all the comments, and I will be very glad to see this get some use!

lonnieezell
lonnieezell approved these changes Oct 19, 2020
View reviewed changes
Copy link
Member

@lonnieezell lonnieezell left a comment

Choose a reason for hiding this comment

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

Agreed. I've been trying to follow the thread, and it sounds like you've got most of the kinks worked out. Huge thanks for taking this one across the finish line!

@michalsn michalsn merged commit 31e943b into codeigniter4:develop Oct 19, 2020
@michalsn
Copy link
Member Author

One more time - thank you @qury!

@robsonpiere
Copy link

robsonpiere commented Oct 27, 2020 •
edited
Loading

Good work!

Is anyone working on the version with PDO ? Isso seria interessante ? Would that be interesting?
I'd like to help.

I have used the driver made available by microsoft (with CI3) and has served me very well.

https://github.com/microsoft/msphpsql/releases

@kenjis kenjis mentioned this pull request May 1, 2024
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mostafakhudair mostafakhudair mostafakhudair left review comments

@paulbalandan paulbalandan paulbalandan left review comments

@qury qury qury left review comments

@MGatner MGatner MGatner approved these changes

@samsonasik samsonasik samsonasik approved these changes

@lonnieezell lonnieezell lonnieezell 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.
9 participants
@michalsn @lonnieezell @samsonasik @MGatner @qury @robsonpiere @paulbalandan @mostafakhudair @ferenc-kurucz