Hi Syed, That is a great idea; however, it is a very hard task. The idea of Tim is great; actually, we already have some sort of hierarchy that is used in “CitrixResourceBase.java”. I would suggest you first removing the unused code, unused variable, and duplicate methods; that would be one PR. You can use a tool called UCdetector to find unused code. To find duplicated code you can use PMD.
One very good example of code duplicity are the methods called “com.cloud.hypervisor.xenserver.resource.CitrixResourceBase.callHostPluginAsync(…)”. After you have cleaned the class, I suggest you analyzing where each remaining method is used and then look for the proper place to put them. It might be a good idea on separating between singletons that are responsible for well-defined tasks such as managing storage, networking, VMs creating and deletion, VMs monitoring and others. If you need any help, please do not hesitate on asking for our help. On Thu, May 19, 2016 at 4:50 PM, Daan Hoogland <daan.hoogl...@gmail.com> wrote: > Syed, > > gogogo. actually it has shrunk to 5k lines since 2012 ;) > > I like your initiative and initial direction. A lot of small steps to > improve the blob have been taken and I would sugest to keep going in small > steps. > > On Thu, May 19, 2016 at 9:44 PM, Tim Mackey <tmac...@gmail.com> wrote: > > > +1 > > > > When I went through this last time, not only was it hard to understand > the > > flows, but the XenServer version management was a pain. Would suggest > > creating a base class which always works (i.e. is independent of > XenServer > > version) for core functions. Then add in that which exists for a specific > > version. Should help greatly with testing IMO. > > > > -tim > > > > On Thu, May 19, 2016 at 2:37 PM, Syed Mushtaq <syed1.mush...@gmail.com> > > wrote: > > > > > Hi All, > > > > > > I would like to refactor CitrixResourceBase class which is responsible > > for > > > communicating with Xenserver. It has grown too long (>5K lines) and has > > > absolutely no testing. > > > > > > In my first pass I want to separate out the functionality buy the > > subsystem > > > it targets (compute, storage, network etc) and will go on from there. > > What > > > do you think? Is anyone working on this currently? > > > > > > Thanks, > > > -Syed > > > > > > > > > -- > Daan > -- Rafael Weingärtner
