Move with_dummy_database... around the implementation of webpack:compile, not the call

[himdel]

This was a fix from #1887, except as it is now, it only works when called via update:ui, not when webpack:compile is being called directly.

Moving to the task implementation. (+ also covering webpack:clobber now)

@NickLaMuro Take a look please? :)
I think EvmRakeHelper should always be available so we don't really need the defined? condition do we? Or was there a different reason?

[miq-bot]

Checked commit https://github.com/himdel/manageiq-ui-classic/commit/f9e77ef1dadf418ea1363e814f50449a9cf31605 with ruby 2.3.3, rubocop 0.47.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🍰

[NickLaMuro]

I think EvmRakeHelper should always be available so we don't really need the defined? condition do we? Or was there a different reason?

Taking a look, but there might have been. Honestly, I forget... we'll see what I remember after I take a look at this.

[NickLaMuro]

Looks fine to me. Left some insight to why the original defined? was there, but not sure it is needed. I think I put it there just so people could use both tasks, and wouldn't confused with it working or not.

[NickLaMuro]

So I think my rational for the defined? is that if you run the rake webpack:compile from this repo (not manageiq proper, and also not the rake app:webpack:compile task), then the task wouldn't work, since it wouldn't load the codebase from manageiq. That said, I haven't tried it recently to confirm if it is a problem or not.

If it is, to avoid duplication, you could possibly do this:

task_proc = lambda do
  Dir.chdir ManageIQ::UI::Classic::Engine.root do
    Rake::Task["webpack:paths"].invoke
    Rake::Task["webpacker:#{webpacker_task}"].invoke
  end
end

if defined?(EvmRakeHelper)
  EvmRakeHelper.with_dummy_database_url_configuration { task_proc.call }
else
  task_proc.call
end

But again, that is only if my theory about the rake webpack:compile from this repo is true.

[himdel]

Great idea with the proc, thanks :). (I was actually thinking gaah, I wish I could add an anonymous function... didn't think of a proc.)

That said, I can verify that running rake webpack:compile from manageiq-ui-classic does load manageiq/lib/tasks/evm_rake_helper.rb.

So.. I guess we don't need it?

[NickLaMuro]

Meh, I would then say you are fine (pun not intended).

That said, does this need to be backported at all? If so, I am hesitant to remove the defined? so close to the build date. But if we are just pushing this to master, then I think it is okay to keep what you have here, and let things break out in the wild if they do.

[himdel]

👍 added gaprindashvili/no.. nothing is actually biting us here, I just had a note about it in my TODO :).

[mzazrivec]

@NickLaMuro is this PR good to go?

[NickLaMuro]

@mzazrivec yeah, I say ship it since it is not being backported. Any issues will probably be found in dev with this, and we can fix it then.