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

[WIP] First H2 Iteration #48

Closed
wants to merge 3 commits into from
Closed

[WIP] First H2 Iteration #48

wants to merge 3 commits into from

Conversation

janheise
Copy link
Contributor

Hi Flavio,

please take a look. It seems to run the tests fine but hasn't been further tested. Please note that I put the schema in quill-jdbc/src/test/resources so that I only have to rebuild this package in case of changes and also because I think it's jdbc-test related. Also, I can easily load it via the classpath for an in-memory instance of H2.

Jan

@codecov-io
Copy link

Current coverage is 99.63%

Merging #48 into master will increase coverage by +0.01% as of c432cae

Powered by Codecov. Updated on successful CI builds.

@@ -35,6 +35,7 @@ lazy val `quill-jdbc` =
libraryDependencies ++= Seq(
"com.zaxxer" % "HikariCP" % "2.3.9",
"mysql" % "mysql-connector-java" % "5.1.36" % "test",
"com.h2database" % "h2" % "1.4.190" % "test",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Formatting

@fwbrasil
Copy link
Collaborator

It's looking great! Could you also review the SqlIdiom to check if there are other things that could be different in h2? We don't have integration tests for all sql features.

@janheise
Copy link
Contributor Author

Thanks for the feedback. I'll get to it asap.

@fwbrasil fwbrasil mentioned this pull request Dec 19, 2015
@fwbrasil
Copy link
Collaborator

@janheise Any news on this? I'm planning to cut a release soon and this PR seems almost ready, it'd be nice to include it.

@janheise
Copy link
Contributor Author

Hi Flavio,

would love to do it, but currently I'm swamped by work. (You catch me
doing work on Sunday morning...)

I'll see what I can do next week.

Jan

On 17.01.2016 10:42, Flavio W. Brasil wrote:

@janheise https://github.com/janheise Any news on this? I'm planning
to cut a release soon and this PR seems almost ready, it'd be nice to
include it.


Reply to this email directly or view it on GitHub
#48 (comment).

@fwbrasil
Copy link
Collaborator

@janheise no worries at all! Thanks for the feedback and good luck! :)

@fwbrasil
Copy link
Collaborator

Would someone be interested in continuing the work on this branch? It's open for too long, it'd be sad to have to close this.

cc/ @jilen @lvicentesanchez @gustavoamigo @godenji

@lvicentesanchez
Copy link
Contributor

I will do it, I always miss proper H2 support in other libraries and this seems almost ready.

@lvicentesanchez lvicentesanchez self-assigned this Feb 19, 2016
@fwbrasil fwbrasil changed the title First H2 Iteration [WIP] First H2 Iteration Feb 22, 2016
@lvicentesanchez
Copy link
Contributor

I have created a local branch from this PR, as it's not possible to push my changes to this PR I will create one as soon as it's ready.

@fwbrasil
Copy link
Collaborator

Ok, I'm closing this one for now.

@fwbrasil fwbrasil closed this Feb 22, 2016
jilen pushed a commit that referenced this pull request Jun 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants