Skip to content

Test that Exec and Query are only called with certain types - #351

Open
cbandy wants to merge 1 commit into
lib:mainfrom
cbandy:driver-contract
Open

Test that Exec and Query are only called with certain types#351
cbandy wants to merge 1 commit into
lib:mainfrom
cbandy:driver-contract

Conversation

@cbandy

@cbandy cbandy commented Mar 15, 2015

Copy link
Copy Markdown
Contributor

Because driver.Value is implemented as an interface{}, its value can actually be any type. It is the responsibility of the sql and driver packages to emit only a few types for these values.

This test ensures those packages are fulfilling their contract in cases we care about.

@johto

johto commented Mar 16, 2015

Copy link
Copy Markdown
Contributor

What was the conclusion with the float32 handling in encode()? Is it dead code?

@cbandy

cbandy commented Mar 16, 2015

Copy link
Copy Markdown
Contributor Author

Yes, it's dead.

@cbandy

cbandy commented Apr 2, 2015

Copy link
Copy Markdown
Contributor Author

@johto would you like me to include a commit that removes the dead code?

@johto

johto commented Apr 4, 2015

Copy link
Copy Markdown
Contributor

@cbandy: Please do.

@cbandy

cbandy commented Apr 4, 2015

Copy link
Copy Markdown
Contributor Author

I've added a commit that removes code for encoding float32 values. The tests for #196, #197, and #198 remain and pass.

@johto

johto commented Apr 8, 2015

Copy link
Copy Markdown
Contributor

I've added a commit that removes code for encoding float32 values. The tests for #196, #197, and #198 remain and pass.

Thanks! I'll try and have a look at this later today.

@cbandy

cbandy commented Apr 22, 2015

Copy link
Copy Markdown
Contributor Author

@johto I think this (or a test like this) is still valuable to include in our suite. What would you like me to do with this PR?

@johto

johto commented Apr 22, 2015

Copy link
Copy Markdown
Contributor

@johto I think this (or a test like this) is still valuable to include in our suite. What would you like me to do with this PR?

I agree, but I haven't had a chance to review it yet. Sorry about that.

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

3 participants