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

Add Dockerfile #157

Merged
merged 2 commits into from Nov 24, 2017
Merged

Add Dockerfile #157

merged 2 commits into from Nov 24, 2017

Conversation

jamescun
Copy link
Contributor

Add a Dockerfile and note in the README about how to use with Docker.

Note: The automated Docker Hub build james/postcodes.io is currently linked to jamescun/postcodes.io. If merged, either Docker Hub james/postcodes.io will need to be altered to ideal-postcodes/postcodes.io or a new Docker Hub repository needs to be created and the README updated.

@coveralls
Copy link

coveralls commented Nov 15, 2017

Coverage Status

Coverage remained the same at 87.29% when pulling 1326b60 on jamescun:master into 6844283 on ideal-postcodes:master.

@cblanc
Copy link
Member

cblanc commented Nov 17, 2017

Thanks James, sorry for the slow response. Will get around to testing this probably on the weekend

Looks great so far

@@ -44,6 +44,24 @@ The import process takes around 10 minutes to complete.
node server.js // Default environment is development
```

### Running with Docker

Postcodes.io is packaged as a Docker container identified on the Docker Hub as `james/postcodes.io`.
Copy link
Contributor

Choose a reason for hiding this comment

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

This would probably need to be changed once merged right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@billinghamj this is addressed in the notes above.

Note: The automated Docker Hub build james/postcodes.io is currently linked to jamescun/postcodes.io. If merged, either Docker Hub james/postcodes.io will need to be altered to ideal-postcodes/postcodes.io or a new Docker Hub repository needs to be created and the README updated.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏼

@billinghamj
Copy link
Contributor

@jamescun Probably worth adding an trailing new line on the dockerfile. Also might be worth making the default port 80? Docker containers always run as root, so no problems with binding < 1024

@cblanc cblanc merged commit 816ebb8 into ideal-postcodes:master Nov 24, 2017
@cblanc
Copy link
Member

cblanc commented Nov 24, 2017

Thanks again, and apologies for the long wait time. Been a bit slammed this past week

I've created a docker hub repository at idealpostcodes/postcodes.io and will update the documentation shortly pending checks to pass to point to that

Also been able to docker run on that repository that locally. Seems to work great. Let me know if I'm missing anything here. Am still a docker / docker hub padawan

Also happy to take further PRs if you wish to amend the dockerfile further

Great stuff and thanks again!

@billinghamj
Copy link
Contributor

👍

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

Successfully merging this pull request may close these issues.

None yet

4 participants