Partitioner multipath fixes #396
Conversation
joseivanlopez
force-pushed
the
partitioner_multipath
branch
from
fc76e1d
to
3d0d756
Compare
| @@ -24,18 +46,17 @@ def initialize(device: nil, table: nil, device_graph: nil) | |||
| @device_graph = device_graph | |||
| end | |||
|
|
|||
| # @macro seeAbstractWidget | |||
| def label | |||
| _("Delete...") | |||
There was a problem hiding this comment.
Add a comment for translators to get the context of the text, e.g.
# TRANSLATORS: push button label (or whatever it actually is...)It might be tricky to translate it correctly...
- If first require of ui_state is done in configurable_blk_devices_table it fails due to cyclic dependency.
joseivanlopez
force-pushed
the
partitioner_multipath
branch
from
896676b
to
d238f77
Compare
| # @return [Array<Y2Storage::BlkDevice>] | ||
| def disk_devices | ||
| disk_types = [Y2Storage::Disk, Y2Storage::Dasd, Y2Storage::Multipath] | ||
| device_graph.disk_devices.select { |d| disk_types.include?(d.class) } |
There was a problem hiding this comment.
Wouldn't this be nicer (more duck-typed and more robust for inheritance) like?
device_graph.disk_devices.select { |d| d.is?(:disk, :dasd, :multipath) }There was a problem hiding this comment.
That's exactly what is? was created for http://www.rubydoc.info/github/yast/yast-storage-ng/master/Y2Storage/Device#is%3F-instance_method
There was a problem hiding this comment.
Of course, I did not remember the multiparams ability of #is?.
|
|
||
| module Y2Partitioner | ||
| module Widgets | ||
| # Class to represent a tab with a list of devices beloging to |
|
|
||
| private | ||
|
|
||
| # Returns a table with all devices used by a MD raid |
There was a problem hiding this comment.
"by a MD array" -> "by the container device"
| # @note An error popup is shown when there is no partition. | ||
| # | ||
| # @return [Boolean] | ||
| def not_empty_partitions_validation |
There was a problem hiding this comment.
Hmm, I find the name (and, in general, how this validations are structured) a little bit confusing. What about, for example?
def delete_validations
if device.is?(:partition)
validate_presence
else
validate_presence && validate_empty_table
end
end
# This is your #not_empty_partitions_validation, but it will never be called if this is a partition
def validate_empty_table
partition_table = device.partition_table
blah, blah
endAs a bonus, you wouldn't need to add support for is?(:partitionable) which is something that does not makes me specially happy.
package/yast2-storage-ng.changes
Outdated
| Wed Oct 25 17:28:48 UTC 2017 - jlopez@suse.com | ||
|
|
||
| - Partitioner: allow to work with multipath devices. | ||
| - Part of fate#318196. |
There was a problem hiding this comment.
Shouldn't bug#1058373 be mentioned here as well?
| # | ||
| # @note Shadowing for BtrFS subvolumes is always refreshed. | ||
| # @see Y2Storage::Filesystems::Btrfs.refresh_subvolumes_shadowing | ||
| # | ||
| # @param device [Y2Storage::BlkDevice] | ||
| def delete_device(device) | ||
| if device.is?(:disk) | ||
| if device.is?(:partitionable) |
There was a problem hiding this comment.
Does is?(:partitionable) work?
PBI: https://trello.com/c/ENYSuS9c/744-3-expert-partitioner-multipath-issues