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

feat(storage) Keychain storage for password. #44

Merged
merged 1 commit into from
Jul 27, 2017

Conversation

wolfeidau
Copy link
Contributor

  • Refactor all login related flags.
  • Added role command line option

@hoegertn
Copy link
Contributor

LGTM but maybe you have a look at how I implemented the role switch and see if you can take some ideas. Especially, the role selection in the login method.

* Refactor all login related flags.
* Added role command line option.
* Added suggestions by @hoegertn to cover a couple of edge cases.

fixes #28 #37 #29
@wolfeidau
Copy link
Contributor Author

@hoegertn Thanks a lot you highlighted a case I had missed and proposed a better name for the flag.

@hoegertn
Copy link
Contributor

👍 looks good. You can reject my PR then.

@wolfeidau wolfeidau merged commit 02c1a1c into master Jul 27, 2017
@hoegertn
Copy link
Contributor

Do you plan to release a new version? I have customers waiting for the role and TLS fix.

@wolfeidau
Copy link
Contributor Author

I have built a release and published it, going to do a bit more testing across all providers tomorrow (Australian time) then update the homebrew tap. Any help testing would be appreciated :)

Cheers

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.

2 participants