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

Refactoring available_images method for miq_ae_orchestrations #12214

Merged
merged 2 commits into from
Nov 30, 2016
Merged

Refactoring available_images method for miq_ae_orchestrations #12214

merged 2 commits into from
Nov 30, 2016

Conversation

pkomanek
Copy link
Contributor

Purpose or Intent

Refactoring the available_images method for the miq_ae_orchestration based on the issue bellow.

Links [Optional]

#12038

@pkomanek
Copy link
Contributor Author

@miq-bot add_labels automate, enhancement

@pkomanek
Copy link
Contributor Author

@miq-bot add_label refactoring

@mkanoor
Copy link
Contributor

mkanoor commented Nov 2, 2016

@pkomanek
Can we add namespaces like we did for the other refactoring

@pkomanek pkomanek changed the title Refactoring available_images method for miq_ae_orchestrations [WIP]Refactoring available_images method for miq_ae_orchestrations Nov 7, 2016
@pkomanek
Copy link
Contributor Author

pkomanek commented Nov 7, 2016

@miq-bot add_label wip

@miq-bot miq-bot added the wip label Nov 7, 2016
@pkomanek pkomanek changed the title [WIP]Refactoring available_images method for miq_ae_orchestrations Refactoring available_images method for miq_ae_orchestrations Nov 16, 2016
@pkomanek
Copy link
Contributor Author

@miq-bot remove_label wip

@miq-bot miq-bot removed the wip label Nov 16, 2016
let(:default_desc) { "<select>" }
let(:default_desc_blank) { "<none>" }
let(:ems) do
@hw1 = FactoryGirl.create(:hardware, :guest_os => 'windows')
Copy link
Contributor

Choose a reason for hiding this comment

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

@pkomanek
can we convert all of these instance variables to lets

it "provides only default value to the image list" do
described_class.new(ae_service).main

expect(ae_service["values"]).to eq(nil => default_desc_blank)
Copy link
Contributor

Choose a reason for hiding this comment

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

@pkomanek
Since this is a new spec file we could also reduce the checks by using a shared example
https://github.com/ManageIQ/manageiq/blob/master/spec/lib/miq_automation_engine/models/miq_ae_git_import_spec.rb#L68
And then calling it within a context as
https://github.com/ManageIQ/manageiq/blob/master/spec/lib/miq_automation_engine/models/miq_ae_git_import_spec.rb#L79

what ever needs to change for the individual test you define in the context and then call it_behvaes_like

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mkanoor thanks for a great tip about shared_examples.

@miq-bot
Copy link
Member

miq-bot commented Nov 17, 2016

Checked commits pkomanek/manageiq@e02d90e~...2aefd7e with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
2 files checked, 0 offenses detected
Everything looks good. 👍

@mkanoor mkanoor merged commit 1ca37af into ManageIQ:master Nov 30, 2016
@mkanoor mkanoor added this to the Sprint 50 Ending Dec 5, 2016 milestone Nov 30, 2016
@pkomanek pkomanek deleted the refactoring_available_images branch December 13, 2016 16:01
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.

4 participants