Mailinglist Archive: yast-devel (87 mails)

< Previous Next >
Re: [yast-devel] Feedback needed! Hack Week project to re-define storage-ng API
On 02/23/2017 09:31 PM, Arvin Schnell wrote:
On Thu, Feb 23, 2017 at 05:45:48PM +0100, Ancor Gonzalez Sosa wrote:

My Hack Week project is in that state in which all the important parts
work and now it's time to decide if it's worth investing a couple of
extra days to finish it all the way down.

Since the ultimate goal is to change how YaST interacts with storage-ng,
I would need the opinion of as many YaST developers as possible.

So please, please, please, take a look to this and give your opinion.
https://github.com/yast/yast-storage-ng/pull/169

Some examples using the swig bindings directly can be better,

Thanks for the corrections.

e.g.:

puts "Ptable type #{a_storage_msdos_ptable.type}" #=> Prints "Ptable type 1"

puts "Ptable type #{a_storage_msdos_ptable.to_s}"

Ok, so let's use another example.

puts "Mount by #{blk_fs.mount_by}" #=> Prints "Mount by 2"

This time, we already used Filesystem#to_s to display the filesystem
type. So we cannot apply the same workaround to this other enum-typed
field in the same class.

As I wrote in the PR description, for some cases we have workarounds and
for others don't. Abusing the #to_s method of Filesystem or
PartitionTable to get the readable version of its type is just a
workaround for one of the many drawbacks explained for whole enums
approach. It does not address the root issues.

or

puts "extended" if [Storage::FsType_EXT2, Storage::FsType_EXT3,
Storage::FsType_EXT4].include?(a_storage_filesystem)

puts "extended" if Storage.ext?(a_storage_filesystem)

Ok, ext? is new (part of my hackweed project). So far ext2 and
ext3 were not implemented.

ptable.type == Storage::PtType_GPT

Storage.gpt?(ptable)

Once again, all those methods mixed altogether directly on the Storage
namespace instead of being the responsibility of more focused classes
look like a workaround to me. Not to mention the risk of name collision
if all the values of all the possible enums need to have a corresponding
method in the same place.

Concerning the nil instead of exceptions:

a_y2storage_unused_partition.filesystem #=> nil

What happens here (nilclass exception?)?

a_y2storage_unused_partition.filesystem.mount_by?

If you call NilClass#mount_by? you will get an exception, of course. Not
sure what's the point of the question, though.

I also thought about placing some classes in sub-namespaces but
mainly to resolve the upcoming name collision of DASD (the
"disk") and DASD (the partition table).

In addition to the class names, this serves as a nice example of my
point above. If at some point we have DASD as a possible value for a
PartitionType enum and as a possible value for a, let's say, Transport
or DiskType enum. Will the Storage.dasd? method be reused for both?
Quite tricky.

The most important question is - do you prefer the proposed approach and
API* or do you prefer to continue accessing directly to libstorage-ng
from YaST?

For me it will make programming in Ruby more difficult since I
have to know two APIs (already mentioned in doc/design-decisions.md).

That's a valid point that affects those who know/develop the libstorage
API and, in addition, has to develop Ruby parts of YaST (i.e. the S
subteam).

In my case, I'm fine with paying that price. Sticking to the libstorage
API also has a price since it's already forcing us to use refinements,
monkey-patching, EnumMapping, custom RSpec matchers[1], etc. I would
gladly pay the price of learning two (very similar) APIs in exchange for
getting rid of all that.

On the other hand, most of the YaST Ruby code (now and in the future)
should be written by other Ruby developers (the S-subteam should be
short-lived[2]). All that people will only have to learn one API.
Thinking in the long term, I would prefer that API to follow Ruby
conventions and to not have the problems described in the PR (like typed
collections, need of downcasting, etc).

Cheers.

[1] On one hand, we have defined custom RSpec matchers to be able to
deal with the "un-rubyness" of objects coming from libstorage. On the
other hand, we cannot use some of the most common RSpec matchers in
situations with some libstorage object involved, because the type
checking imposed by Swig blows the matchers away.

[2] I expect the subteam itself to be relatively short-lived, not the
members (I hope we will all survive to storage-ng). :-)

--
Ancor González Sosa
YaST Team at SUSE Linux GmbH
--
To unsubscribe, e-mail: yast-devel+unsubscribe@xxxxxxxxxxxx
To contact the owner, e-mail: yast-devel+owner@xxxxxxxxxxxx

< Previous Next >