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

Tests for SqlSrv #1048

Closed
wants to merge 2 commits into from
Closed

Tests for SqlSrv #1048

wants to merge 2 commits into from

Conversation

milo
Copy link
Member

@milo milo commented Apr 16, 2013

No description provided.

@hrach
Copy link
Contributor

hrach commented Apr 16, 2013

Well, I don't like the first commit. Why is mysql default? Default means what? Imo default could be only some ansi sql, not mysql.

@milo
Copy link
Member Author

milo commented Apr 16, 2013

IMO, the nearest to ANSI is PostgreSQL. But here, I feel default as another 'unknown' driver. And because MySQL have unusual quoting, driver-specific tests are failing. It is very practical when adding another driver.

It is important to have something to default. Otherwise are some assertions silently skipped for new driver.

@milo
Copy link
Member Author

milo commented Apr 16, 2013

And in practical way... almost all driver specific assertions are about quotation. There are a few databases which use the same quotations. Maybe default should be sqlsrv due to [ ] quoting.

@milo
Copy link
Member Author

milo commented Apr 17, 2013

Let me know if I should changed it and/or rebase after this commit storm ;)

@hrach
Copy link
Contributor

hrach commented Apr 17, 2013

Please rebase :)

@dg
Copy link
Member

dg commented Apr 17, 2013

What about this 39bcb22?

@milo
Copy link
Member Author

milo commented Apr 17, 2013

Perfect! Thanks. I'll runs tests on MSSQL. If something wrong apprears, I'll add it to #1056 which I'm working on now.

@milo milo closed this Apr 17, 2013
@milo milo deleted the database/tests branch April 17, 2013 19:07
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.

None yet

3 participants