Re: [libvirt-java][PATCH] make Flags used to manipulate Domain accessible from outside of the package

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

 



[FTR, I've noticed the same thing when I started to use the Java
 bindings. Back then, this made me wonder whether the bindings are
 actually used by someone, because the constants are rather useless
 without being public and nobody objected...]

At Fri, 21 Mar 2014 14:05:26 -0600,
Yoshikazu Nojima wrote:
> 
> Hi,
> I'd like to submit a small patch to libvirt-java.
> This patch make Flags used to manipulate Domain accessible from
> outside of the package

NACK as I'm planning on fixing this in a different way.

With the current approach, there are a few reasons why I'm wishing to
avoid this change:

1. the names are too long

   [we don't have to use a prefix for variable names, since there are
    other means of scoping in Java (namely namespaces and classes/interfaces)]

2. using int as a flags argument is not type safe and it entices the
   use of magic numbers in the code

So, the plan is to use enums and variable arguments for the API and
deprecate the use of the constructors and methods with an integer flag
argument; similar to the patch I've posted here[1].

If you want to help push the Java bindings into this direction, you're
more than welcome.

> I'm new to libvirt-java development process, please let me know if I
> miss something.

At first, thanks for your contribution!

There's just one minor thing, we prefer to receive patches as single
messages, not as attachments. Best way is to use "git send-email", see
http://libvirt.org/hacking.html#patches.

And it would be nice if you could indicate the project/binding you're
working on in the subject using the phrase "[java]" which you can
achieve using "git config format.subjectprefix 'java] [PATCH'" when
using "git send-email".

[1]: http://www.redhat.com/archives/libvir-list/2014-February/msg00879.html

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]