Skip to content

Commit

Permalink
finish_client: improved and documented the API.
Browse files Browse the repository at this point in the history
`title` and `steps` are used after all, I was wrong.
  • Loading branch information
mvidner committed Nov 28, 2014
1 parent 8d1d7aa commit ffd402f
Showing 1 changed file with 29 additions and 11 deletions.
40 changes: 29 additions & 11 deletions library/general/src/lib/installation/finish_client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -62,22 +62,40 @@ def run

protected

# Abstract method to write configuration
# Write configuration.
def write
raise NotImplementedError, "Calling abstract method 'write'"
end

# Abstract method to provide information about client
# @return [Hash] with keys
#
# * **`"steps"`** [Integer] ---
# number of client steps,
# * **`"when"`** [Array<Symbol>] ---
# modes when client make sense; if missing, then run always
# * **`"title"`** [String] ---
# used to display to user what is happening.
# Restrict in which modes it should run.
# @return [Array<Symbol>, nil]
# Valid values are `:autoinst`, `:autoupg`, `:installation`,
# `:live_installation`, and `:update`. NOTE that these values
# are NOT consistent with the names used in {Yast::ModeClass Mode}.
# By default it returns `nil`, meaning to run always.
def modes
nil
end

# @return [Integer] the number of client steps.
def steps
1
end

# @return [String] a title used to display to the user what is happening.
def title
raise NotImplementedError, "Calling abstract method 'title'"

This comment has been minimized.

Copy link
@jreidinger

jreidinger Nov 28, 2014

Member

hmm, it is not consistent with inst_proposal which have backup when hash do not contain such key.

This comment has been minimized.

Copy link
@mvidner

mvidner Nov 28, 2014

Author Member

Are you sure? Can you suggest an alternative?

This comment has been minimized.

Copy link
@jreidinger

jreidinger Nov 28, 2014

Member

damn, I mean implementation in inst_finish - https://github.com/yast/yast-installation/blob/master/src/clients/inst_finish.rb#L442
so if we return nil, then fallback is used.

This comment has been minimized.

Copy link
@mvidner

mvidner Nov 28, 2014

Author Member

I see. But it will say "Calling step foo_finish". So I think it is fair to demand supplying a better text for new code.

This comment has been minimized.

Copy link
@jreidinger

jreidinger Nov 28, 2014

Member

OK, it is fair, as we do need to be backward compatible. So lets merge it.

end

private

This comment has been minimized.

Copy link
@jreidinger

jreidinger Nov 28, 2014

Member

why not protected? if you need to extend it, it is better to use this one as base and just add key like

def info
  res = super
  res["my_info"] = "info"
  res
end

This comment has been minimized.

Copy link
@mvidner

mvidner Nov 28, 2014

Author Member

Good point, fixed that.


# Adapt the metadata for inst_finish API
def info
raise NotImplementedError, "Calling abstract method 'info'"
{
"when " => modes,
"steps" => steps,
"title" => title
}
end
end
end

0 comments on commit ffd402f

Please sign in to comment.