Mount point asterisk

[jreidinger]

how it looks:

[coveralls]

Coverage increased (+0.002%) to 97.037% when pulling 42b8c68 on mount_point_asterisk into cb106f8 on master.

[joseivanlopez]

Why is the mode important?

[jreidinger]

because in installation it is useless as it shows only if it is part of proposal, so shows newly mounted partition not in staging, which is something very different. Also help text does not reflect it.

[joseivanlopez]

But during installation, all new mount points are always active, so the asterisk will be not shown in that case.

[jreidinger]

I think only when they are in staging, so situation that they are not in staging cannot happen?

[joseivanlopez]

Maybe we could avoid the high complexity error in codeclimate with a implementation like follows:

def device_filesystem_mount_point
  # TRANSLATORS: Mount point information, where %s is replaced by a mount point
  res = format(_("Mount Point: %s"), blk_device.filesystem_mountpoint || "")

  if Yast::Mode.normal && blk_device.filesystem && blk_device.filesystem.mount_point
  	mount_point = blk_device.filesystem.mount_point
  	# TRANSLATORS: note appended to mount point if mount point is not now mounted
  	res += _(" (not mounted)") unless mount_point.active?
  end

  res
end

[jreidinger]

I am not sure that it helps. If we want to reduce maybe something like

if Yast::Mode.normal && blk_device&.filesystem&.mount_point&.active? == false
  res += _(" (not mounted)")
end

[joseivanlopez]

This version is difficult to read for me. What about adding a new auxiliary method that checks whether it is necessary to add the ending note?

[joseivanlopez]

And here, we try something like:

def mount_point_value(device)
  fs = filesystem(device)

  return "" if fs.nil? || fs.mount_path.nil?

  res += "(*)" if Yast::Mode.normal && !fs.mount_point.active?

  res
end

[joseivanlopez]

BTW, comparing with old partitioner, I see it more clear when I see "/mount/point *". Current version "/mount/point(*)" looks a little bit strange to me, but it is only a personal feeling.

[jreidinger]

fine for me with changing it to " *"

[joseivanlopez]

LGTM