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

Better: Allow to set host in load-pg-dump script #2523

Merged
merged 1 commit into from Oct 9, 2020

Conversation

ylecuyer
Copy link
Contributor

@ylecuyer ylecuyer commented Oct 2, 2020

In CONTRIBUTING.md, one installation method is with docker but the load-pg-dump script defaults to using postgresql locally with the unix socket.

I have added the -H option to set the hostname for postgresql commands. It still works as before is no -H option is given

pg_host=
if [ -n "$host" ]; then
pg_host="-h $host"
fi
Copy link
Member

Choose a reason for hiding this comment

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

Would using one variable pg_host and using localhost as default for that be simpler than what we are doing here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it would be easier indeed but it would break the current behavior of the script. So it is up to you project owners and script users to decide what to do here ;)

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure I understand how would it break the current behavior. are we talking about PGHOST?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the default is to use the unix socket instead of localhost

@sonalkr132 sonalkr132 merged commit 48ba37e into rubygems:master Oct 9, 2020
@sonalkr132
Copy link
Member

Thank you @ylecuyer

@sonalkr132 sonalkr132 temporarily deployed to staging October 9, 2020 05:34 Inactive
@sonalkr132 sonalkr132 temporarily deployed to production October 9, 2020 05:53 Inactive
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.

None yet

2 participants