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

implement postgres naming strategy; fixes #215 #218

Merged
merged 3 commits into from
Feb 27, 2016

Conversation

godenji
Copy link
Contributor

@godenji godenji commented Feb 27, 2016

No description provided.

@godenji
Copy link
Contributor Author

godenji commented Feb 27, 2016

@fwbrasil @gustavoamigo @lvicentesanchez @jilen please review.

p.s. I've noticed builds in last couple of PRs have been failing on MySQLAsync/PostgresAsync. Rebuild passes though, not sure if temporary or something to look into.

@fwbrasil
Copy link
Collaborator

@godenji if you see a build error, please report the error message and stack trace here before restarting the job. We're investigating these errors.

@@ -80,11 +80,15 @@ trait CamelCase extends NamingStrategy {
}
object CamelCase extends CamelCase

trait PostgresEscape extends Escape {
override def column(s: String) = if (s.startsWith("$")) s else default(s)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not else super.column(s)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

never mind, clean build solved AbstractMethodError I was getting, will update PR.

I'll grab stack trace if spurious build error occurs again.

@fwbrasil
Copy link
Collaborator

👍

Approved with PullApprove

fwbrasil added a commit that referenced this pull request Feb 27, 2016
implement postgres naming strategy; fixes #215
@fwbrasil fwbrasil merged commit 56a2d04 into zio:master Feb 27, 2016
@fwbrasil
Copy link
Collaborator

@godenji Thanks for the quick fix! :)

@godenji
Copy link
Contributor Author

godenji commented Feb 27, 2016

@fwbrasil speaking of quick fixes there's actually a bunch of cleanup work to do (various typos in source files and readme phrasing/grammar) before you give your ScalaDays talk in Berlin 😄

I'll try to make note of them as they come up, get distracted by the interesting work and then forget the little details...

@fwbrasil
Copy link
Collaborator

@godenji awesome! looking forward to it!

@fwbrasil
Copy link
Collaborator

@godenji I've just noticed that this change wasn't reflect in the readme, could you fix it?

@godenji
Copy link
Contributor Author

godenji commented Mar 1, 2016

@fwbrasil done, see #236

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.

2 participants