Skip to content
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

Adds a config key to set the default login authType. Closes #5585 #5591

Closed
wants to merge 1 commit into from

Conversation

martinlingstuyl
Copy link
Contributor

Closes #5585

Adds a config key to set the default login authType.

Also re-added a space behind the chili icon before the device code message.
In bash the missing space looks ugly.

@Adam-it Adam-it self-assigned this Nov 18, 2023
@martinlingstuyl
Copy link
Contributor Author

Updated this to add an extra test case

Copy link
Member

@Adam-it Adam-it left a comment

Choose a reason for hiding this comment

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

code is ✅
tested ✅
you rock 🤩

one strange behavior I noticed not connected to your change is
image

I changed the default to password and hit login. I got an error which is fine buuuut if CLI is interactive shouldn't I get a prompt for that if it is required 😉.
I guess a good one for a follow up issue 🙂 don't you agree?

@Adam-it
Copy link
Member

Adam-it commented Nov 21, 2023

ready to merge 🚀

@Adam-it
Copy link
Member

Adam-it commented Nov 21, 2023

merged manually 👏

@Adam-it Adam-it closed this Nov 21, 2023
@martinlingstuyl
Copy link
Contributor Author

code is ✅ tested ✅ you rock 🤩

one strange behavior I noticed not connected to your change is image

I changed the default to password and hit login. I got an error which is fine buuuut if CLI is interactive shouldn't I get a prompt for that if it is required 😉. I guess a good one for a follow up issue 🙂 don't you agree?

Hi @Adam-it, yeah, that's the reason I created this issue: #5636, as we're talking about conditionally required options.
But now it appears that we first have to research zod :)

@martinlingstuyl martinlingstuyl deleted the authtype-config branch November 21, 2023 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enhancement: add a config key to set the default login authType
2 participants