Yeah, it has to go into 4.9. :) Unless no one cares about Hyper-V. ________________________________________ From: Rafael Weingärtner <rafaelweingart...@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 “_”. The point is that in some other language the use of “_” 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. For 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 could 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 people 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.tutkow...@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ärtner <rafaelweingart...@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 or > 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 > “_”(underline) before variable names; even though Apache 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’s 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 by > 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 it 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=6928953&tag=1 > [2] https://github.com/ivansalvadori/gsonld > > > On Fri, May 20, 2016 at 3:30 AM, Daan Hoogland <daan.hoogl...@gmail.com> > wrote: > > > Guys, we should rename fields that have improper names. I do not agreee > > 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.tutkow...@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 if > they > > > are not as expected. > > > > > > In the meanwhile, I plan to change the variable names back that were > > > changed in PR #816. > > > > > > Additional thoughts on how this should be addressed long term? > > > > > > Thanks! > > > Mike > > > ________________________________________ > > > From: Anshul Gangwar <anshul.gang...@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 with > > > 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 names 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 > > > ========== > > > This e-mail may contain privileged and confidential information which > 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ärtner > -- Rafael Weingärtner
