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

fix missing tokenCodeFn #181

Merged
merged 2 commits into from
Mar 20, 2019
Merged

fix missing tokenCodeFn #181

merged 2 commits into from
Mar 20, 2019

Conversation

gregory
Copy link
Contributor

@gregory gregory commented Mar 10, 2019

I was missing the tokenCodeFn and allow to pass mfa serial through environment variable.

@gojko
Copy link
Member

gojko commented Mar 10, 2019

Hi, instead of readline, let's make this another command line argument?

@gregory
Copy link
Contributor Author

gregory commented Mar 11, 2019

What about I update to have both, so that it ll fallback to readline if not present in arguments?

@gojko
Copy link
Member

gojko commented Mar 11, 2019

that sounds ok to me.

@gregory
Copy link
Contributor Author

gregory commented Mar 11, 2019

addressed your suggestion @gojko ! Let me know if you need anything else.
Cheers.

@gojko
Copy link
Member

gojko commented Mar 13, 2019

hi - thanks for this, looks good. I will test and merge over the weekend.

@gojko
Copy link
Member

gojko commented Mar 18, 2019

Hi - I merged your code and tried to add some specs around it so we can maintain it easier. can you please try if this branch still works for you: https://github.com/claudiajs/claudia/tree/gregory-fix_bug_with_mfa

if it does what you need, we can merge everything into master

@gregory
Copy link
Contributor Author

gregory commented Mar 19, 2019

sorry for the delay @gojko - that was a good idea.
Code is cleaner & still works for me.
Thanks, let me know if i can help with anything else.

@gojko gojko merged commit f19a2d8 into claudiajs:master Mar 20, 2019
@gojko
Copy link
Member

gojko commented Mar 20, 2019

@gregory thanks, will release this later today then

@gojko
Copy link
Member

gojko commented Mar 21, 2019

I can't publish this yet because the current aws-sdk release breaks claudia, I've reported a bug there and waiting on them to release it. as soon as that's done, I'll re-run the tests again and push a new release out

@gregory
Copy link
Contributor Author

gregory commented Mar 22, 2019

wow - crazy - can you link the bug you reported so that we can track status here?
Thanks for the head's up!

@gojko
Copy link
Member

gojko commented Mar 22, 2019

@gregory the bug was aws/aws-sdk-js#2588. I released claudia 5.4.2 just now, with your patch, so upgrading to the latest version should have this PR merged in

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