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.

Sign up for GitHub

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

Jump to bottom

build(dockerfiles): split dockerfile into production and dev #1325

Merged
merged 4 commits into from
Mar 18, 2021
Merged

build(dockerfiles): split dockerfile into production and dev #1325

merged 4 commits into from
Mar 18, 2021

Conversation

orbitalsqwib
Copy link
Contributor

Problem

The existing Dockerfile caters to both development and production builds, which increases the container startup time for developers and overall complexity.

Closes #178

Solution

  • split Dockerfile into production and development dockerfiles
  • removed unnecessary conditionals for production dockerfile
  • updated docker-compose to use development dockerfile

liangyuanruo
liangyuanruo previously requested changes Mar 18, 2021
View reviewed changes
Copy link
Contributor

@liangyuanruo liangyuanruo left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this! Have you tried deploying to the staging environment? I think the rename to Dockerfile.production might break the Travis pipeline / Elastic Beanstalk deployment.

@orbitalsqwib
Copy link
Contributor Author

Thanks for tackling this! Have you tried deploying to the staging environment? I think the rename to Dockerfile.production might break the Travis pipeline / Elastic Beanstalk deployment.

I'm working on the build pipeline for edu right now, so i'll take a look at that too. Thanks for the heads up!

LoneRifle
LoneRifle requested changes Mar 18, 2021
View reviewed changes
Copy link
Contributor

@LoneRifle LoneRifle left a comment

Choose a reason for hiding this comment

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

rebase changes on top of develop. in addition, consider using Dockerfile and Dockerfile.dev for deployments and localhost builds respectively

@orbitalsqwib
build(dockerfiles): split dockerfile into production and dev
ab2fb30 
- split dockerfile into production and development dockerfiles
- removed unnecessary
conditionals
- updated docker-compose to use development dockerfile

fix #178
@orbitalsqwib
fix(dockerfiles): add back font install for dev dockerfile
f72444a 
re #178
@orbitalsqwib
build(ci): rename dockerfiles for ci pipeline
4e78116 
- Renamed Dockerfiles to take advantage of default dockerfile configuration in ci pipeline

re #178
@orbitalsqwib orbitalsqwib force-pushed the build/dockerfile-shard branch from eab9aee to 4e78116 Compare March 18, 2021 08:53
@orbitalsqwib
chore(dockerfiles): remove unused args
132ff62 
- Removed the unused BUILD_MODE argument

re #178
LoneRifle
LoneRifle approved these changes Mar 18, 2021
View reviewed changes
Copy link
Contributor

@LoneRifle LoneRifle left a comment

Choose a reason for hiding this comment

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

lgtm

@LoneRifle LoneRifle dismissed liangyuanruo’s stale review March 18, 2021 09:08

review comments taken into account in subsequent commits

@LoneRifle LoneRifle merged commit c68ba77 into develop Mar 18, 2021
@LoneRifle LoneRifle deleted the build/dockerfile-shard branch March 18, 2021 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LoneRifle LoneRifle LoneRifle approved these changes

@liangyuanruo liangyuanruo Awaiting requested review from liangyuanruo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Shard Dockerfile into development and production variants
3 participants
@orbitalsqwib @liangyuanruo @LoneRifle