Skip to content

Conversation

@diemol
Copy link
Member

@diemol diemol commented Aug 24, 2018

  • Fixes Add HEALTHCHECK entry to Dockerfiles #691 by documenting how to add a health check when starting the containers. We won't add a health check into the docker images since it is not possible to configure the intervals for it, and we don't have data to choose values that work for everyone.

  • Improved templates for new issues and PRs.

  • By placing an X in the preceding checkbox, I verify that I have signed the Contributor License Agreement

do
case "$1" in
--host)
HOST="$2"
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you do HOST="${2:-localhost}"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea, I was just copying and pasting from other script. Just made the changes and I will push them now. Thanks @tnguyen14

README.md Outdated
exec $cmd
```
**Note:** If needed, replace `localhost` and `4444` for the correct values in your environment. Also, this script is polling indefinitely, you might want
to teak it and establish a timeout.
Copy link
Member

Choose a reason for hiding this comment

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

s/teak/tweak/

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @cgoldberg

@diemol diemol merged commit a328437 into master Aug 26, 2018
@diemol diemol deleted the wait-for-it branch August 26, 2018 10:34
@neverstew
Copy link

@diemol - sorry just pulled the code and all looks good. I was going off of the dockerfile preview on https://hub.docker.com/r/selenium/base/~/dockerfile/ (this is still showing old code)

@diemol
Copy link
Member Author

diemol commented Aug 30, 2018

No worries :)

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.

6 participants