Skip to content
This repository has been archived by the owner on Feb 14, 2021. It is now read-only.

add loginshell type #84

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

add loginshell type #84

wants to merge 2 commits into from

Conversation

edrex
Copy link
Collaborator

@edrex edrex commented Nov 14, 2016

satisfies that the current user's login shell is set to some value

satisfies that the current user's login shell is set to some value
@edrex
Copy link
Collaborator Author

edrex commented Nov 15, 2016

thinking it would be nice if this added the entry to /etc/shells if it isn't already there (maybe just add https://github.com/mattly/dotfiles/blob/master/bork/types/shells.sh as well)

Copy link
Collaborator

@frdmn frdmn left a comment

Choose a reason for hiding this comment

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

Nice! 👍

@edrex
Copy link
Collaborator Author

edrex commented Jan 3, 2017

Proposal: combine types/shells.sh from mattly/dotfiles with this, a single type with an optional second argument to set as current user shell:

ok shell /usr/local/bin/zsh login

With this form it would both ensure there is a line in /etc/shells and that the current user shell is set to it. Without the optional login param it would just check /etc/shells.

@mattly wdyt?

@mattly
Copy link
Owner

mattly commented Feb 21, 2017

The code here looks good. Pursuant to the discussion about merging with the shell type from my dotfiles, I'm inclined to agree that merging this into that as an option is probably the best path. I've been meaning to write tests for that and merge it here, but thanks to 👶 I haven't even been keeping my dotfiles in sync these days.

The reasoning behind merging this to shell is that a) it's only for the current user, b) that's probably the 90% use case one might expect from this, and c) while we could add an option to specify the user for whom this is for, the users type has its own option to specify a user's shell (I found some resources on programmatically creating users on Darwin, hopefully I can add that).

I'd like to say I'll be able to take a stab at merging them this week/end, but I've been telling myself I'll look at these PRs "really soon" for six weeks now, so... If you want to take a stab at it, let me know and I'll leave it to you, otherwise I'll do it when I can. It shouldn't be too hard.

@mattly mattly added this to the 0.12.0 release milestone Jan 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants