Mailing-List: contact dev-help@cloudstack.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@cloudstack.apache.org
MIME-Version: 1.0
In-Reply-To: <1463762855653.9613@netapp.com>
References: <3497C24D-C446-4068-A156-D1920DA05848@persistent.co.in>
	<1463720005877.40747@netapp.com>
	<CAGQtxvaaX1CE1gTW+10bvZyMCy7u3LAB_Wxrnv9oVE=Jym_gDw@mail.gmail.com>
	<CAG97rac_snu1-MCk4v458+a5r49dtE4L=nD0PT3k3jRmzZbSRg@mail.gmail.com>
	<1463761654834.31029@netapp.com>
	<CAG97raddqQkY3GzPtBq_Bgn5ahYDUzzT3dY1ZkiPY_NJfzPcRw@mail.gmail.com>
	<1463762855653.9613@netapp.com>
Date: Fri, 20 May 2016 13:49:46 -0300
Message-ID: <CAG97radG-0LHhTPQdsYM7XzLRaNWHvvAqT0iGxmX109ebmewWg@mail.gmail.com>
Subject: Re: Variable renaming in classes meant for Agents
From: =?UTF-8?Q?Rafael_Weing=C3=A4rtner?= <rafaelweingartner@gmail.com>
To: "dev@cloudstack.apache.org" <dev@cloudstack.apache.org>
Content-Type: multipart/alternative; boundary=94eb2c0441f4d5fa93053348e0f5
archived-at: Fri, 20 May 2016 16:49:59 -0000

--94eb2c0441f4d5fa93053348e0f5
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

I think that if we say we support Hyper-V we should care about it.

On Fri, May 20, 2016 at 1:47 PM, Tutkowski, Mike <Mike.Tutkowski@netapp.com=
>
wrote:

> Yeah, it has to go into 4.9. :) Unless no one cares about Hyper-V.
> ________________________________________
> From: Rafael Weing=C3=A4rtner <rafaelweingartner@gmail.com>
> Sent: Friday, May 20, 2016 10:42 AM
> To: dev@cloudstack.apache.org
> Subject: Re: Variable renaming in classes meant for Agents
>
> You are right Mike about the =E2=80=9C_=E2=80=9D. The point is that in so=
me other language
> the use of =E2=80=9C_=E2=80=9D makes sense, whereas in Java it does not, =
at least not the
> way it has being used in ACS.
>
> We have code conventions, it can be found in [1]. The problem is that it =
is
> a bit outdated and I think it could benefit from some others tutorials. F=
or
> instance, a clear and simple tutorial explaining what is a test case and
> showing how to create a proper test case; I am referring to the
> unit/integration test case that we write using Junit and other tools.
>
> Also, we lack some information on how to prepare code to be tested.
> Once we have that kind of standard defined and tutorials written, we coul=
d
> work out ways to educate our community. It is not a problem not to know
> those things; we cannot expect everyone to know how to use every single
> methodology and technology that is out there. But, we can help people to
> learn, that is the point of a community, it should be a place where peopl=
e
> exchange ideas and experience in a way that benefits everyone.
>
> As soon as you open the PR, please let us know, so we can review it and
> help you get it merge as soon as possible. I think this is something that
> should go in the 4.9 release.
>
>
> On Fri, May 20, 2016 at 1:27 PM, Tutkowski, Mike <
> Mike.Tutkowski@netapp.com>
> wrote:
>
> > It sounds like most people don't like a preceding "_" for member
> variables
> > and that is fine. Do we have any formal Java coding standards for
> > CloudStack, by the way? I'm not aware of any.
> >
> > The main problem here, though, is that this particular piece of code is
> > super fragile, so it would be great to harden it up.
> >
> > I'm going to open a PR and revert the names in those changed "Command"
> > files for 4.9. That will solve the immediate problem.
> > ________________________________________
> > From: Rafael Weing=C3=A4rtner <rafaelweingartner@gmail.com>
> > Sent: Friday, May 20, 2016 9:12 AM
> > To: dev@cloudstack.apache.org
> > Subject: Re: Variable renaming in classes meant for Agents
> >
> > Hi guys,
> > I agree with Daan that if class fields have improper (not descriptive o=
r
> > not suitable) names we should change them. However, I do not find the
> > change (on variable names) introduced by PR #816 good. It added an
> > =E2=80=9C_=E2=80=9D(underline) before variable names; even though Apach=
e CloudStack has a
> > lot of that currently, I think that is a pattern we should avoid.
> >
> > Your ideas to use annotations to avoid relying on variable names are
> great;
> > but, let=E2=80=99s not re-create the well here. There is a research [1]=
 that has
> > been conducted in 2014 that tackled exactly that problem; the proposal
> > presented in [1] decoupled client and server sides from variable name b=
y
> > using semantic annotations. The concept, the formalization and the
> > experiments are all presented in paper [1]. The serialization and
> > deserialization core of the proposal presented in [1] can be found in
> [2].
> >
> > The idea of decoupling our web APIs from variable names is great, but i=
t
> is
> > something that will require some effort to be fully and properly
> > implemented. If you wish to push that forward count on me.
> >
> > [1] http://ieeexplore.ieee.org/xpls/abs_all.jsp?arnumber=3D6928953&tag=
=3D1
> > [2] https://github.com/ivansalvadori/gsonld
> >
> >
> > On Fri, May 20, 2016 at 3:30 AM, Daan Hoogland <daan.hoogland@gmail.com=
>
> > wrote:
> >
> > > Guys, we should rename fields that have improper names. I do not agre=
ee
> > > with the statement at all. Your two solutions to the serialisation
> hazard
> > > are both acceptable to me. leaving a non compliant or non explanatory
> > name
> > > in because it slipped through the nets at some points does not seem
> > > acceptable to me. We must improve are code.
> > >
> > > On Fri, May 20, 2016 at 6:53 AM, Tutkowski, Mike <
> > > Mike.Tutkowski@netapp.com>
> > > wrote:
> > >
> > > > Thanks for sending out this e-mail, Anshul.
> > > >
> > > > This is a bit of a strange situation because we need to make sure
> > people
> > > > are either aware of the fact that properties in Command classes are
> > > > serialized (and not change existing variable names) or come up with=
 a
> > > less
> > > > fragile way of choosing property names when sending data (perhaps
> using
> > > > annotations).
> > > >
> > > > At the very least, we should have comments in these classes
> indicating
> > > the
> > > > dangers of changing property names. It might also be beneficial to
> have
> > > > unit tests in place that expect certain variable names and assert i=
f
> > they
> > > > are not as expected.
> > > >
> > > > In the meanwhile, I plan to change the variable names back that wer=
e
> > > > changed in PR #816.
> > > >
> > > > Additional thoughts on how this should be addressed long term?
> > > >
> > > > Thanks!
> > > > Mike
> > > > ________________________________________
> > > > From: Anshul Gangwar <anshul.gangwar@accelerite.com>
> > > > Sent: Thursday, May 19, 2016 10:47 PM
> > > > To: dev@cloudstack.apache.org
> > > > Subject: Variable renaming in classes meant for Agents
> > > >
> > > > Hi,
> > > >
> > > > We should not allow renaming of variables in classes which ends wit=
h
> > > > Command and TO. As these objects are meant to be consumed by Agents=
.
> > > >
> > > > Agents may not be written in java so relying on these variable name=
s
> to
> > > > get the info. One such example is Hyper-V agent.
> > > >
> > > > Hyper-V support is currently broken as there are some variables
> renamed
> > > in
> > > > PR https://github.com/apache/cloudstack/pull/816.
> > > >
> > > > Regards,
> > > > Anshul
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > DISCLAIMER
> > > > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> > > > This e-mail may contain privileged and confidential information whi=
ch
> > is
> > > > the property of Accelerite, a Persistent Systems business. It is
> > intended
> > > > only for the use of the individual or entity to which it is
> addressed.
> > If
> > > > you are not the intended recipient, you are not authorized to read,
> > > retain,
> > > > copy, print, distribute or use this message. If you have received
> this
> > > > communication in error, please notify the sender and delete all
> copies
> > of
> > > > this message. Accelerite, a Persistent Systems business does not
> accept
> > > any
> > > > liability for virus infected mails.
> > > >
> > >
> > >
> > >
> > > --
> > > Daan
> > >
> >
> >
> >
> > --
> > Rafael Weing=C3=A4rtner
> >
>
>
>
> --
> Rafael Weing=C3=A4rtner
>


--=20
Rafael Weing=C3=A4rtner

--94eb2c0441f4d5fa93053348e0f5--