On Wed, Dec 03, 2014 at 11:07:34AM +0100, Arvin Schnell wrote:
Bindings --------
I have swig generated bindings for Ruby, Python and Perl5. The repository contains a few examples for each of them:
https://github.com/aschnell/libstorage-bgl-eval/tree/master/bindings
Hi Arvin, I tried to review the Ruby bindings with the goal of making them readable to a Rubyist. The method naming convention, in camelCase, looks out of place in Ruby. It should be possible to autoconvert it to snake_case via swig -ruby -autorename example.i http://www.swig.org/Doc3.0/Ruby.html#Ruby_nn27 - sda1 = gpt.createPartition("/dev/sda1") + sda1 = gpt.create_partition("/dev/sda1") Similarly, though less automatable, it would be good to rename getters and setters: http://www.swig.org/Doc3.0/Ruby.html#Ruby_nn31 - partition.getNumber() + partition.number Abbreviations can be useful to save typing and screen space but IMHO they should be avoided in class names: - Storage::BlkDevice::find(...) + Storage::BlackDevice.find(...) (A week ago I and Balazs were trying to debug string handling in Ruby VM. It took me several minutes to realize that what I thought was "capability" meant "capacity", and "terminal" was in fact "terminator". They were declared as "capa" and "term" without an explanatory comment.) Small improvements in style in the examples: - Storage::Disk::create(device_graph, "/dev/sda") + Storage::Disk.create(device_graph, "/dev/sda") - print "foo\n" + puts "foo" Please avoid booleans in the API, they are hard to understand even if you have learned the domain and the API before. I haven't seen a single mention od Ruby symbols in the Swig docs, I guess custom typemaps would be needed for that :-( - sda.getDescendants(false) + sda.descendants(:direct) # or Storage::DESCENDANTS_DIRECT For polymorphism, we have these 6 casts in Storage::Device: (which return nil if the instance is not of that type) #castToDisk #castToPartitionTable #castToPartition #castToLvmVg #castToLvmLv #castToFilesystem While the ideal OO design is to avoid casts and use method polymorphism, I don't know how doable it is in this case. A more rubyish method names would be #to_disk #to_partition_table #to_lvm_vg -- Martin Vidner, Cloud & Systems Management Team http://en.opensuse.org/User:Mvidner Kuracke oddeleni v restauraci je jako fekalni oddeleni v bazenu