Skip to content

Change default shell if installing ZSH #325

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

Merged
merged 4 commits into from
Dec 9, 2022
Merged

Change default shell if installing ZSH #325

merged 4 commits into from
Dec 9, 2022

Conversation

jacobwoffenden
Copy link
Contributor

Changes default shell to /bin/zsh

Resolves #324

@samruddhikhandale
Copy link
Member

Added my thoughts to the original issue #324 (comment)

Hi @jacobwoffenden 👋 ,

As a heads up, we're discussing splitting up the common-utils Feature: #67 where we have discussed of creating a new feature for Zsh.

When using the common-utils feature, and setting installZsh to true (default behaviour), I think it would be useful to change the default shell for the determined ${USERNAME}

This is a good point and makes sense to me. However, making this change now could be a little disruptive for the users who expects bash as their default shell when they use this common-utils Feature. Also, most of the dev container images rely on this common-utils Feature. The community using these images have also been expecting bash as the default shell. We could add a command to change the shell to bash for the image but again it would be little contradictory.

I wonder if we could hold on making this change in #325 for now as it will be soon addressed by #67 (comment)? I can add this suggestion to ensure that zsh is configured for the default user when the zsh Feature is used. We could also add a new isConfigured: true property for the Feature so that anyone could disable and simply install zsh. (That'll be useful for the dev container images)

@jacobwoffenden what do you think?

@jacobwoffenden
Copy link
Contributor Author

@microsoft-github-policy-service agree

Copy link
Member

@samruddhikhandale samruddhikhandale left a comment

Choose a reason for hiding this comment

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

Thank you, appreciate it! Left some comments

Added test scenario
Bumped feature version
Copy link
Member

@samruddhikhandale samruddhikhandale left a comment

Choose a reason for hiding this comment

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

Looks amazing! Thank you 👏

@samruddhikhandale samruddhikhandale merged commit aae9de7 into devcontainers:main Dec 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Common Utils - If installing ZSH, set as default shell for user
2 participants