Skip to content

Add password flag for authentication #43

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Feb 27, 2020
Merged

Add password flag for authentication #43

merged 3 commits into from
Feb 27, 2020

Conversation

BrainBlasted
Copy link
Collaborator

For T692

thebaer
thebaer requested changes Sep 6, 2019
View reviewed changes
Copy link
Member

@thebaer thebaer left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this, Chris. Here's a few things that need to be addressed based on reading through the code.

Going forward, let's just be sure to test our changes for regressions before opening pull requests. And let me know if anything's unclear.

@thebaer
Copy link
Member

thebaer commented Sep 9, 2019

Perfect, code looks good now 👍

Last thing I'd suggest is removing the redundant string() cast on line 408 in commands.go -- then I'll test this out.

@BrainBlasted
Add password flag for authentication
c6af292 
For T692
@BrainBlasted
commands: Fix up the interactive auth flow
f2bb9f5 
Fixes up the interactive authorization workflow so that
pass is reassigned properly.

For T692
@BrainBlasted
commands: Remove redundant string() cast on pass
bd378a8 
When logging in we had a cast that was made redundant, since
`pass` is now a string.
@BrainBlasted
Copy link
Collaborator Author

Done and rebased.

@BrainBlasted BrainBlasted requested a review from thebaer October 9, 2019 01:37
@thebaer
Copy link
Member

thebaer commented Feb 27, 2020

Looks great, thanks!

@thebaer thebaer merged commit 62e0352 into master Feb 27, 2020
@thebaer thebaer deleted the T692 branch February 27, 2020 18:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thebaer thebaer thebaer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@BrainBlasted @thebaer