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 Docker configuration files #14

Merged
merged 7 commits into from
Nov 23, 2021
Merged

Add Docker configuration files #14

merged 7 commits into from
Nov 23, 2021

Conversation

kj84park
Copy link
Member

@kj84park kj84park commented Nov 6, 2021

What is this PR for?

Overview or reasons

  • Added a Docker configuration and description so that users can easily check the application behavior.

Tasks

  • Add docker execution environment configuration.

Comment on lines 3 to 14
rp:
container_name: rp-server
image: kj84park/rp-server:0.0.1
ports:
- "8080:8080"
restart: always
fido2:
container_name: fido2-server
image: kj84park/fido2-server:0.0.1
ports:
- "8081:8081"
restart: always

Choose a reason for hiding this comment

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

In my opinion, it would be better if the images are built from the local Dockerfiles, instead of pulled from dockerhub.

Choose a reason for hiding this comment

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

For the Dockerfiles themselves, we can use multi-stage docker builds so the build process is isolated from the local environment

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you so much for the suggestion! 👍
Let me check.

Copy link
Member Author

@kj84park kj84park Nov 21, 2021

Choose a reason for hiding this comment

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

@ric-sapasap
I have changed the settings according to your comments. thank you.
If you have any more comments, please.

Choose a reason for hiding this comment

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

Looks amazing. Thank you very much @kj84park!

Copy link

@haxelion haxelion left a comment

Choose a reason for hiding this comment

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

Nothing to add :-)

@kj84park kj84park merged commit 3a75508 into line:main Nov 23, 2021
@kj84park kj84park deleted the etc/add-docker-settings branch November 23, 2021 05:21
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

3 participants