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

Updated error message as per #322 #325

Merged
merged 2 commits into from Aug 11, 2016
Merged

Updated error message as per #322 #325

merged 2 commits into from Aug 11, 2016

Conversation

wjdhamilton
Copy link

It would probably be better in the long run if a gist was created in the hanami repo or a blog post in hanamirb to properly explain the console issue, but I think this will suffice for now given that the ROM upgrade is on its way.

James Hamilton added 2 commits June 28, 2016 08:16
Error message updated for to reflect the fact that the database may
exist, or there may be a live psql console connected to it.
@coveralls
Copy link

coveralls commented Jun 28, 2016

Coverage Status

Coverage increased (+0.2%) to 94.284% when pulling ff938eb on wjdhamilton:master into a639406 on hanami:master.

@cllns
Copy link
Member

cllns commented Aug 10, 2016

This addresses #322 (in title here but putting in this comment so it's referenced in that issue)

Sorry for the delay here. I don't think we should link to #250 in the error message since it's only tangentially related (it's for dropdb not createdb). Seems potentially more confusing than helpful.

Can we change the error message to just createdb: database creation failed. If the database exists, then its console may be open. ? I think that's clear enough. What do you think, @wjdhamilton?

@wjdhamilton
Copy link
Author

Hi Sean, yes. Um...only one small problem which is that I mistakenly deleted my branch in the intervening period. One of those times I really should have read the warning message. Anyway, I will effect those changes in a new repo and get the pull request in.

@cllns
Copy link
Member

cllns commented Aug 10, 2016

No problem! It's a small enough fix and GitHub cached the diff (on the Changes page) so you can re-do it from that.
Thanks!

@jodosha jodosha merged commit 287a4ab into hanami:master Aug 11, 2016
@jodosha jodosha self-assigned this Aug 11, 2016
@jodosha jodosha added this to the v0.7.0 milestone Aug 11, 2016
@jodosha
Copy link
Member

jodosha commented Aug 11, 2016

@wjdhamilton I merged it, thanks!

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

Successfully merging this pull request may close these issues.

None yet

4 participants