Refactored to a set of install scripts to avoid adding further complexity to the base Dockerfile and make it easier to disable tools #385
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
vyadh
commented
Sep 2, 2024
myoung34
previously requested changes
Sep 5, 2024
There was a problem hiding this comment.
I used some free time to add basic tests to this project to make sure that all the logic is captured as assertions
If you'd like to squash all these commits down into one and rebase against master you'll see those run against this and it will help make sure that no user facing behavior is changed
|
Ah, great stuff. Thank you for doing that. I'll get on it. |
Additionally, this change: - Configures upfront to reduce number of apt updates - Removes use of lsb-release to allow simplification and avoid unnecessary dependencies in the base image (i.e. Python)
vyadh
force-pushed
the
install-scripts
branch
from
a017e43 to
0bcea86
Compare
|
I'll fix the shell check stuff next. I didn't notice there was an action for that. I was relying on IntelliJ telling me issues! |
|
🤷 |
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.
Refactored the
Dockerfile.baseout into a set of scripts that install the various tools in mostly the same way, just in a more maintainable and legible way. For my purposes the motivation was to make it easier to strip things out and create a minimal image. I've also changed a few things such as the removal oflsb-release, which seemed unnecessary, mainly due to it simplifying the refactoring and to avoid a Python dependency if its not otherwise required. I've tried to make the refactoring steps somewhat easier to follow / diffable via the various commits to make it easier to review.It does a few less calls to
apt updategiven the up-front sources configuration, and so is a touch faster, but it's not significant (a few 10s of seconds on my machine).I've tested this and for my purposes it works, though I'm not using many of the tools that are installed in this image.
Somewhat related to #380.