Readability cleanup & minor fixes #3
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
- Fixed potential issues in Dockerfile - Optimized mkdir RUN instruction - Cleaned it up, for easier reading
- Fixed error, where "apt-get update" should be followed by "apt-get install" on the same RUN instruction, to prevent issues if debian pushes updates between instructions.
- Added very newcomer friendly comments to the bootstrap.sh script. - Replaced whitespaces with tab based indentation. I recommend using editor settings instead, if you wish to keep it 2 spaces long.
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.
I redid all my improvements and split it all up into 4 commits.
First I added a badly needed .dockerignore file. It will save you a lot of time when we don't add the entire git history to the build containers.
Secondly I did a cleanup of the main Dockerfile, including the changes that somehow made my issues disappear.
Thirdly I fixed your Debian base Dockerfile, that did not follow some best practices regarding the apt cache & layering. Simply put, if Debian pushes updates to the package repositories at the wrong time (no matter how unlikely) things could break.
Lastly I changed all white spaces into tabs, added empty new line separators and minor readability tweaks.on most scripts and all the rest of the Dockerfiles.
If you prefer 2 spaces long indentation, I recommend using an editor that supports setting a custom length (which only works if you use tabs). Thus to make things easier for potential contributors I recommend tabs.
As you might see in the comments of the bootstrap.sh file, I did figure out the role of the loop and the if statement.
But I could never figure out why it needed to be run 3 times, instead of just 2. Changing it to a 2 times loop resulted in images of exact same size and contents. I also discovered a minor time saver that could be easily implemented if the muslbase-runtime and muslbase-static-runtime only was created the last loop run.
All of that I will address in a second pull request, after we have had time to discuss this one. I figured that was a slightly larger issue and a slightly greater chance of me missing something.