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

Extract methods cli #289

Merged
merged 2 commits into from
Oct 18, 2024
Merged

Extract methods cli #289

merged 2 commits into from
Oct 18, 2024

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Oct 16, 2024

extracted from #223

Cli#run method is a bit complicated, so extracting these methods to trim it down

lib/floe/cli.rb Outdated
elsif opts[:credentials_file_given]
File.read(opts[:credentials_file])
else
{}
Copy link
Member

Choose a reason for hiding this comment

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

@kbrock this looks like a functional change (nil vs {} when credentials not passed). Was this intentional? Not a big deal since in Context we do @credentials = credentials || {} anyway just curious why the change

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm overly sensitive right now to us making conversions (e.g.: making JSON.parse(x) if x.kind_of?(String) )

I'll undo this. Though I kinda like it better this way.

code climate complained run had too many lines
@agrare agrare merged commit 6db8fe1 into ManageIQ:master Oct 18, 2024
5 checks passed
@kbrock kbrock deleted the extract_methods_cli branch October 18, 2024 20:30
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.

2 participants