Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Swift db drivers #526

Closed
wants to merge 12 commits into
from

Conversation

Projects
None yet
2 participants

Update the Swift adapters for Sequel to use

https://github.com/deepfryed/swift-db-mysql
https://github.com/deepfryed/swift-db-postgres
https://github.com/deepfryed/swift-db-sqlite3

All three have been released to Rubygems and the corresponding specs all pass.

I have to look into all the pending tests for swift and try to get most of them to pass.

Thanks

Doesn't this add ::Sequel::Error instead of ::Swift::Error?

Owner

deepfryed replied Jul 15, 2012

good catch, thanks

Owner

jeremyevans commented Jul 25, 2012

Briefly reviewing this now. Some notes:

We generally don't do $stderr.puts for explanatory error messages in exceptions. It should be obvious to users that if you get a LoadError when requiring swift/db/mysql, you need to install a library, and the documentation you added to opening_databases.rdoc should cover that.

You are removing ::SwiftError from Postgres::CONVERTED_EXCEPTIONS instead of replacing it with ::Swift::Error. What's the reasoning behind that?

Other than those things, this looks good, and I should be able to merge and test it tomorrow.

Thanks for the help!

Owner

jeremyevans commented Jul 25, 2012

Also, looking at the new drivers, it doesn't look like dbic++ is used anymore? Is that correct? If so, what's the status of dbic++ (I'm the OpenBSD maintainer of the dbic++ port)?

We generally don't do $stderr.puts for explanatory error messages in exceptions.

removed the LoadError puts

"You are removing ::SwiftError from Postgres::CONVERTED_EXCEPTIONS instead of replacing it with ::Swift::Error. What's the reasoning behind that?"

I must have done it because Swift::Error required loading the swift/db/postgres first. I've just rearranged the require order and put that code back in sequel/adapters/swift/postgres

it doesn't look like dbic++ is used anymore?

Yes, it doesn't. Decoupling swift from the binary adapters was in the pipeline for a while and this rewrite lead to more cleaner and easier to maintain codebase.

what's the status of dbic++

I'm still maintaining dbic++ as it is still a library I use for writing c++ code.

Owner

jeremyevans commented Jul 26, 2012

Thanks for the changes and the info. I'll try to get this merged and tested tomorrow.

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