Skip to content

Conversation

@jreidinger
Copy link
Contributor

No description provided.


private

def report(title, step, substeps: 0, current_substep: 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not making it public and use it directly in the installer.rb?

I mean,

...

def install
  ...
  Yast::Installation.destdir = "/mnt"
  # Imaging we have a collection of steps or so where partitioning could be...
  # 
  # class Partitioning < InstallationStep
  #   def start_label
  #     _("Partitioning target disk")
  #   end
  # 
  #   def perform
  #     Yast::WFM.CallFunction("inst_prepdisk", [])
  #   end
  #  
  #   ...
  steps = [partitioning, package_installation, bootloader_installation]

  # So...

  progress = InstallationProgress.new(@dbus_obj, steps.size, logger: logger)
  
  steps.each_with_index |step, index| do
    progress.report(step.start_label, index)
    step.perform
    progress.report(step.end_label, index + 1)
  end

  ...
end

...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

well, I am still not sure whose responsibility it is. Also progress needs to know about number of steps so it can report it. So question is who owns responsibility for proper messaging. Still not sure what would be the best solution

Copy link
Contributor

Choose a reason for hiding this comment

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

Also progress needs to know about number of steps so it can report it

Right. That's why I put the "steps.size" in the InstallationProgress constructor. To tell it the number of steps when creating it, instead of having an internal constant that must be in sync with the installer.rb.

Anyway, let's continue playing around. Thank you!

@jreidinger jreidinger merged commit 0e5bae2 into master Jan 10, 2022
@jreidinger jreidinger deleted the bootloader-install branch January 10, 2022 11:53
joseivanlopez added a commit that referenced this pull request Mar 27, 2025
Changes on top of latest LVM features
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.

3 participants