Move kube/openshift specific template code into its provider directory. #79
Conversation
|
@simon3z @moolitayer Please review. This is part of ManageIQ/manageiq#15523 , of which the data migration change has already been merged in ManageIQ/manageiq-schema#35 |
|
@moolitayer @zakiva can you reivew? |
| @@ -0,0 +1,69 @@ | |||
| module ManageIQ::Providers::Kubernetes::Shared::ContainerManager::ContainerTemplate | |||
There was a problem hiding this comment.
This file is an exact copy of the code removed from app/models/container_template.rb in ManageIQ/manageiq#15523 so 👍 for the code as well as including this in this repo per ManageIQ/manageiq#15523 (comment).
As for the new shared path currently we have lots of examples of shared code (ContainerManagerMixin, EventCatcherMixin, some of metrics_capure, scanning/job & entities_mapping all from a quick look)
So while I think the new folder is a good idea, having only ContainerTemplate there will do more to confuse.
There was a problem hiding this comment.
I generally think of the openshift provider as a "subclass" of kubernetes provider.
Certainly Openshift the technology is-a Kubernetes.
Now technically we don't always subclass, we frequently have both openshift and kubernetes include a shared mixin — but I'm not sure it needs drawing attention to.
(And if it does, I'd rather use "Mixin" suffix)
-0 on need for shared/ folder, and -0.5 on deeper nesting. For God's sake, we're already working in
manageiq/plugins/manageiq-providers-kubernetes/app/models/manageiq/providers/kubernetes/container_manager/
😢 It's not a veto, if others think it's good I'll play along.
There was a problem hiding this comment.
Also, as things are now broken until these 3 PRs land, I'm in favor of merging sooner and figuring out naming later.
There was a problem hiding this comment.
I would rather not merge as is(especially since ManageIQ/manageiq#15523 was not yet merged).
Seems app/models/manageiq/providers/kubernetes/container_manager/container_template_mixin.rb is more consistent with what we currently have.
lfu
force-pushed
the
refactor_container_manager
branch
from
f161868 to
8214e13
Compare
|
Checked commit lfu@8214e13 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
|
Since I merged ManageIQ/manageiq#15523, I am going to merge the dependent PRs to avoid things going red. |
there's no particular reason, just update for update sake https://github.com/ManageIQ/more_core_extensions/compare/179bf40..e5b4501 - Added Ruby 2.7 support [[ManageIQ#79](ManageIQ/more_core_extensions#79)] - Added Process#pause, Process#resume, and Process#alive? [[ManageIQ#73](ManageIQ/more_core_extensions#73)] array added * `#compact_map` - Collect non-nil results from the block array added `#tabular_sort` - Sorts an Array of Hashes by specific columns hierarchy added `#descendant_get` - Returns the descendant with a given name the two breaking changes: - **BREAKING**: Moved Object#descendant_get to Class#descendant_get [[ManageIQ#75](ManageIQ/more_core_extensions#75)] - **BREAKING**: Removed deprecated Enumerable#stable_sort_by [[ManageIQ#76](ManageIQ/more_core_extensions#76)] a minor header output change was made that hasn't been released yet to make tableize more markdown compliant see ManageIQ/linux_admin#221
there's no particular reason, just update for update sake https://github.com/ManageIQ/more_core_extensions/compare/179bf40..e5b4501 - Added Ruby 2.7 support [[ManageIQ#79](ManageIQ/more_core_extensions#79)] - Added Process#pause, Process#resume, and Process#alive? [[ManageIQ#73](ManageIQ/more_core_extensions#73)] array added * `#compact_map` - Collect non-nil results from the block array added `#tabular_sort` - Sorts an Array of Hashes by specific columns hierarchy added `#descendant_get` - Returns the descendant with a given name the two breaking changes: - **BREAKING**: Moved Object#descendant_get to Class#descendant_get [[ManageIQ#75](ManageIQ/more_core_extensions#75)] - **BREAKING**: Removed deprecated Enumerable#stable_sort_by [[ManageIQ#76](ManageIQ/more_core_extensions#76)] a minor header output change was made that hasn't been released yet to make tableize more markdown compliant see ManageIQ/linux_admin#221
there's no particular reason, just update for update sake https://github.com/ManageIQ/more_core_extensions/compare/179bf40..e5b4501 - Added Ruby 2.7 support [[ManageIQ#79](ManageIQ/more_core_extensions#79)] - Added Process#pause, Process#resume, and Process#alive? [[ManageIQ#73](ManageIQ/more_core_extensions#73)] array added * `#compact_map` - Collect non-nil results from the block array added `#tabular_sort` - Sorts an Array of Hashes by specific columns hierarchy added `#descendant_get` - Returns the descendant with a given name the two breaking changes: - **BREAKING**: Moved Object#descendant_get to Class#descendant_get [[ManageIQ#75](ManageIQ/more_core_extensions#75)] - **BREAKING**: Removed deprecated Enumerable#stable_sort_by [[ManageIQ#76](ManageIQ/more_core_extensions#76)] a minor header output change was made that hasn't been released yet to make tableize more markdown compliant see ManageIQ/linux_admin#221
Part of ManageIQ/manageiq#15523.
Blocks ManageIQ/manageiq-providers-openshift#28.
@miq-bot add_label refactoring