Skip to content
This repository has been archived by the owner on Jul 25, 2023. It is now read-only.

Add verify_credentials and params_for_create #136

Merged
merged 1 commit into from
Oct 1, 2019

Conversation

agrare
Copy link
Member

@agrare agrare commented Oct 1, 2019

Add a method to return the parameters required to add an SCVMM provider
and a method to verity credentials with those parameters.

ManageIQ/manageiq#18818

Add a method to return the parameters required to add an SCVMM provider
and a method to verity credentials with those parameters.
@miq-bot
Copy link
Member

miq-bot commented Oct 1, 2019

Checked commit agrare@648b60d with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🏆

@@ -20,6 +20,61 @@ def self.description
@description ||= "Microsoft System Center VMM".freeze
end

def self.params_for_create
@params_for_create ||= {
Copy link
Member

Choose a reason for hiding this comment

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

No constant like the other providers?

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 thought a constant plus a class method could be confusing, didn't want people to use the constant instead. Plus the rest of this class uses the @ ||= style to prevent allocating these on every method call

Copy link
Member

Choose a reason for hiding this comment

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

Ok...I'd almost rather we change the other ones with constants to be consistent, then.

@Fryguy Fryguy merged commit 796e0c0 into ManageIQ:master Oct 1, 2019
@Fryguy Fryguy added this to the Sprint 122 Ending Oct 14, 2019 milestone Oct 1, 2019
@agrare agrare deleted the add_verify_credentials branch October 1, 2019 19:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants