support for openshift_postgresql #5

Merged
merged 2 commits into from Nov 3, 2015

Projects

None yet

2 participants

@spradnyesh
Contributor

OPENSHIFT_POSTGRESQL_DB_URL starts with postgresql:// instead of postgres://

@spradnyesh
Contributor

OPENSHIFT_POSTGRESQL_DB_URL starts with postgresql:// instead of postgres://

@pupeno pupeno merged commit f8ecba7 into pupeno:master Nov 3, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pupeno
Owner
pupeno commented Nov 3, 2015

Thank you for the contribution.

@spradnyesh
Contributor

:)
when can you push a new version?

@pupeno
Owner
pupeno commented Nov 3, 2015

@spradnyesh it's done. Just be aware of the change in groupid, it's now com.carouselapps/to-jdbc-uri.

@spradnyesh
Contributor

ohh, cool. thanks :)
and thanks too for the caution

@pupeno
Owner
pupeno commented Nov 3, 2015

There was a small issue, so I fixed it and released it as 0.4.1.

@spradnyesh
Contributor

@pupeno: i saw what you've done, and i also saw the docs for "case" to understand what you're doing
but i didn't understand why you called my commit an "issue" (no hard feelings, just wanting to learn if what you did is more idiomatic). the reason i say what i did wasn't an issue is because all of your existing tests, as well as my new one passed
thanks in advance (for the explanation) :)

@pupeno
Owner
pupeno commented Nov 3, 2015

When you pass a list as the matching parameter to a case, such as (1 2 3), case will compare the value you are dispatching on to each of the members of the list, 1, 2 and 3 in this case. With your patch it was comparing the schema against the strings "postgres" and "postgresql" but it was also comparing it against the function or as your list was (or "postgres" "postgresql"). I don't think the schema would have ever matched the function or, but it was a needless comparison.

I think a potential issue would have been if under some circumstances comparing a string against a function would raise an exception (something that wouldn't surprise me).

@spradnyesh
Contributor

hmm, i understand. thanks again :)

@pupeno
Owner
pupeno commented Nov 3, 2015

Thank you.

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