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

Refactor opsmanager handling #521

Merged
merged 1 commit into from
Nov 25, 2019
Merged

Conversation

ekohl
Copy link
Member

@ekohl ekohl commented Nov 30, 2018

This removes the trivial private classes in favor of doing it all in the main class. This also means moving all params into the main class which has the side effect that the REFERENCE.md will be much more useful when generated.

@alexjfisher
Copy link
Member

I don’t inheritly see a problem with using the install/config/service pattern. I’d have probably gone for simplifying the subclasses (removing unnecessary variable reassignment/bringing into scope) and removal of params.pp (as it contains no OS specific conditional logic)

@dhoppe dhoppe added the needs-work not ready to merge just yet label Jun 5, 2019
@dhoppe
Copy link
Member

dhoppe commented Jun 5, 2019

@ekohl Could you please take care of the RuboCop issues?

@vox-pupuli-tasks
Copy link

Dear @ekohl, thanks for the PR!

This is pccibot, your friendly Vox Pupuli GitHub Bot. I noticed that your pull request contains merge conflict. Can you please rebase?

You can find my sourcecode at voxpupuli/vox-pupuli-tasks

This removes the trivial private classes in favor of doing it all in the
main class. This also means moving all params into the main class which
has the side effect that the REFERENCE.md will be much more useful when
generated.
@ekohl
Copy link
Member Author

ekohl commented Nov 21, 2019

Updated.

I don’t inheritly see a problem with using the install/config/service pattern.

Given how trivial this is now, this is much easier to follow so in this case I think it's worth it.

@ekohl ekohl removed tests-fail needs-work not ready to merge just yet labels Nov 25, 2019
@ekohl ekohl merged commit 37dd6f0 into voxpupuli:master Nov 25, 2019
@ekohl ekohl deleted the refactor-ops-handling branch November 25, 2019 15:32
@nmaludy nmaludy mentioned this pull request Feb 10, 2020
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.

5 participants