[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Ovirt-devel] [patch] Even Better Exceptions For ruby-libvirt bindings

Mohammed Morsi wrote:
> Updated and attached. Good news is everything compiles fine and 
> everything seems to work. Due to request, I did not add the libvirt 
> exception structure to the ruby-libvirt exception object, rather the 
> message field is copied over. For the next revision the code, domain, 
> level, other message fields, and anything else anyone needs will also be 
> included. Enjoy!

> +
> +
> +// define additional errors here
> +static VALUE e_Error;                   // Error - generic error
> +static VALUE e_ConnectionError;         // ConnectionError - error durring connection establishment
> +static VALUE e_DefinitionError;         // DefinitionError - error during data definition
> +static VALUE e_RetrieveError;           // RetrievalError - error during data retrieval
> +static VALUE e_OperationError;          // OperationError - error during other various operations

>From a quick glance, it's unclear to me what the difference between e_Error and
e_OperationError is; what exactly is your intent here?

> +
> +    /*
> +     * Class Libvirt::VirError - core Libvirt error structure wrapper
> +     */
> +    c_vir_error = rb_define_class_under(m_libvirt, "VirError", rb_cObject);

What is this used for?  There is only the static at the top of the file, and
then this define_class, but no additional methods or subclasses.  What's the
intent here?

Besides the cleanup I've mentioned here, this is a definite improvement over the
current exception implementation.  Once this is fixed, it will get an ACK from me.

Chris Lalancette

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]