-
Notifications
You must be signed in to change notification settings - Fork 6
Collection of Improvements #24
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
Collection of Improvements #24
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you make use of versions.sh in this file to not repeat the version numbers?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately no, I think. This file belongs to the devcontainer s-core-devcontainer itself, not to the local feature s-core-local (which provides tools for which no existing devcontainer feature can be used). It would be an encapsulation violation to do so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does sound wrong to have versions in two places. But I don't understand enough of the setup to propose any solution here.
Also the versions should be easily accessible somehow for unrelated, non-devcontainer usecases. That's probably a topic in itself.
duplicate, and it is already in the base-image anyway
otherwise, re-builds of the container may "suddenly" contain unexpected versions of relevant tools
this is really *a lot*
otherwise, re-builds of the container may "suddenly" contain unexpected versions of relevant tools
Since these tools are really important, we should definitely check that the versions are as expected.
Following https://code.visualstudio.com/remote/advancedcontainers/persist-bash-history, this implements an across-container-restarts-persistent bash history.
8b55ce5 to
c0ac659
Compare
|
Getting |
| sudo chown "${USERNAME}:${GROUPNAME}" "${COMMANDHISTORY_DIR}" | ||
| sudo chmod 755 "${COMMANDHISTORY_DIR}" | ||
|
|
||
| # Create .bash_history file if it doesn't exist and set permissions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me test that one. It has zsh support, which may be not a bad idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried it, but it fails to install
0.109 ===========================================================================
0.110 Activating feature 'shell-history'
0.110 User: vscode User home:
0.110 ***********************************************************************************
0.110 *** Require _REMOTE_USER and _REMOTE_USER_HOME to be set (by dev container CLI) ***
0.110 ***********************************************************************************
0.110 ERROR: Feature "Shell History" (ghcr.io/stuartleeks/dev-container-features/shell-history) failed to install!
Apparently, there are additional requirements for this feature. For this little advantage, it seems to be too complicated.
d4fe1db to
6e28bbb
Compare
Here, I have collected a few improvements to the DevContainer which I think should go in before we call this a 1.0.0.