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

Deduplicate app/controllers/vm_common.rb by using MoreShowMixins #375

Open
martinpovolny opened this issue Feb 14, 2017 · 3 comments
Open

Comments

@martinpovolny
Copy link
Member

martinpovolny commented Feb 14, 2017

For some of the methods the code is the same.

I see a problem with 'show_timeline':

  1. seems that the URL generated there for breadrumb is wrong -- it contains db that comes from get_rec_cls and that returns a class.
  2. some of the show_timeline methods call find_by_id_filtered that should be probably called everywhere instead
  3. there are a bunch of alianses (such as :image_timeline,, some are used from toolbars)
  4. there's extra code to deal with @explorer being true

warning: if inside a module A you use:

included do
  include Mixins::MoreShowMixins
end

and then try to redefine a method from MixingsMoreShowMixins, you'll fail as the method from the included mixin will get called, not the one in module A.

When done, look at app/controllers/container_controller.rb it also has it's implementation of the methods from MoreShowMixins but slightly differently arranged.

@martinpovolny martinpovolny changed the title Use MoreShowMixins in app/controllers/vm_common.rb Deduplicate app/controllers/vm_common.rb by using MoreShowMixins Feb 14, 2017
@miq-bot miq-bot added the stale label Dec 25, 2017
@JPrause
Copy link
Member

JPrause commented Jan 28, 2019

@martinpovolny is this still a valid issue? If yes, lease remove the stale label. If not can you close.
If there's no update by next week, I'll be closing this issue.

@JPrause
Copy link
Member

JPrause commented Feb 6, 2019

Closing issue. If you feel the issue needs to remain open, please either reopen or let me know and it will be reopened.
@miq-bot close_issue

@miq-bot miq-bot closed this as completed Feb 6, 2019
@martinpovolny martinpovolny reopened this Feb 7, 2019
@JPrause
Copy link
Member

JPrause commented Feb 7, 2019

@miq-bot remove_label stale

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants