-
Notifications
You must be signed in to change notification settings - Fork 1
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
Use upstream base image #44
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ake sure they get generated
…octane flavor is dtected/selected, temporarily use a flag to generate the upstream file
…ic from the repository maintaining its previous custom image
…sh when generating files for upstream based docker image
…, while still using fly-apps/laravel-docker Dockerfile logic
…, while still using fly-apps/laravel-docker Dockerfile logic
Dockerfile
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Discussion: See discourse post.
TLDR: we're porting the fly-apps/laravel-docker functionality over to this package, so that this package may create a Dockerfile that does not use the custom image built by that repository, but uses an upstream image instead.
WHAT AND WHY
Revise Dockerfile template to create a Dockerfile that uses an upstream base image. There are three reasons for this:
HOW
Well super easy! fly-apps/laravel-docker actually uses an upstream image as base, so we can simply merge the Dockerfile's logic it creates with our existing Dockerfile's logic, along with the necessary config files used by that Dockerfile to dockerfile-laravel.
How To Review
Ok! There're a lot of files right? lots.
The easiest way to review this PR would be to see if it works for a local Laravel app one might have:
pwd
---this should get the directory where the local dockerfile-laravel can be retrieved fromphp <path\to\dockerfile-laravel-project>\dockerfile-laravel generate --upstream-base-image
docker build -t my_sample_app .
docker run -p 8080:8080 my_sample_app
Hopefully everything works out well!
Lastly if you really wanna be thorough, here are the important files that got them changes:
Primary Logic
Additional Templates
There're a lot of these.
resources/views/fly
- should contain subdirectories and files that are templates to be rendered into the user's .fly directoryThe Tests
These are not quite essential. but. essential, too at the same time! Feel free to check one by one.