5:45 p.m.
Hi!
Do we really need the AddVmPoolCommand class? What I see currently:
1. VdcActionType.AddVmPool is never used.
2. CommonVmPoolWithVmsCommand extends AddVmPoolCommand, but this inheritance is useless:
a. AddVmPoolCommand.executeCommand() is never called from
CommonVmPoolWithVmsCommand.executeCommand(), the same work is done by
AddVmPoolWithVmsCommand.getPoolId().
b. AddVmPoolCommand.getAuditLogTypeValue() is never called.
c. AddVmPoolCommand.getValidationGroups() does the same as
AddVmPoolWithVmsCommand.getValidationGroups() and may be merged to
UpdateVmPoolWithVmsCommand.getValidationGroups() if needed (I'm not sure).
d. AddVmPoolCommand() constructor code may be merged to CommonVmPoolWithVmsCommand()
constructor.
3. The hierarchy where UpdateVmPoolWithVmsCommand is inherited indirectly from
AddVmPoolCommand seems illogical.
So can AddVmPoolCommand be safely removed?
--
Shmuel
1:35 p.m.
----- Original Message -----
From: "Shmuel Melamud" <smelamud(a)redhat.com>
To: devel(a)ovirt.org
Sent: Sunday, March 29, 2015 5:45:52 PM
Subject: [ovirt-devel] Useless AddVmPoolCommand class
Hi!
Do we really need the AddVmPoolCommand class? What I see currently:
1. VdcActionType.AddVmPool is never used.
2. CommonVmPoolWithVmsCommand extends AddVmPoolCommand, but this inheritance
is useless:
a. AddVmPoolCommand.executeCommand() is never called from
CommonVmPoolWithVmsCommand.executeCommand(), the same work is done by
AddVmPoolWithVmsCommand.getPoolId().
b. AddVmPoolCommand.getAuditLogTypeValue() is never called.
so you can also remove the AuditLogType values used there, and the translations (if not in
use)
c. AddVmPoolCommand.getValidationGroups() does the same as
AddVmPoolWithVmsCommand.getValidationGroups() and may be merged to
UpdateVmPoolWithVmsCommand.getValidationGroups() if needed (I'm not sure).
d. AddVmPoolCommand() constructor code may be merged to
CommonVmPoolWithVmsCommand() constructor.
just make sure current behavior is kept
3. The hierarchy where UpdateVmPoolWithVmsCommand is inherited
indirectly
from AddVmPoolCommand seems illogical.
So can AddVmPoolCommand be safely removed?
yes
--
Shmuel
_______________________________________________
Devel mailing list
Devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel
5:33 p.m.
On Mar 30, 2015, at 12:35 , Omer Frenkel <ofrenkel(a)redhat.com> wrote:
----- Original Message -----
> From: "Shmuel Melamud" <smelamud(a)redhat.com>
> To: devel(a)ovirt.org
> Sent: Sunday, March 29, 2015 5:45:52 PM
> Subject: [ovirt-devel] Useless AddVmPoolCommand class
>
> Hi!
>
> Do we really need the AddVmPoolCommand class? What I see currently:
>
> 1. VdcActionType.AddVmPool is never used.
> 2. CommonVmPoolWithVmsCommand extends AddVmPoolCommand, but this inheritance
> is useless:
> a. AddVmPoolCommand.executeCommand() is never called from
> CommonVmPoolWithVmsCommand.executeCommand(), the same work is done by
> AddVmPoolWithVmsCommand.getPoolId().
> b. AddVmPoolCommand.getAuditLogTypeValue() is never called.
so you can also remove the AuditLogType values used there, and the translations (if not
in use)
> c. AddVmPoolCommand.getValidationGroups() does the same as
> AddVmPoolWithVmsCommand.getValidationGroups() and may be merged to
> UpdateVmPoolWithVmsCommand.getValidationGroups() if needed (I'm not sure).
> d. AddVmPoolCommand() constructor code may be merged to
> CommonVmPoolWithVmsCommand() constructor.
just make sure current behavior is kept
> 3. The hierarchy where UpdateVmPoolWithVmsCommand is inherited indirectly
> from AddVmPoolCommand seems illogical.
>
> So can AddVmPoolCommand be safely removed?
>
yes
There's also a corresponding annoyance in UI when you try to remove the pool.
There is a "Remove" in the top command bar on Polls main tab, but it only tells
you that you have to detach VMs first. You can do that in the bottom subtab…but when you
remove the last one (or all at once) it says it will also remove the pool and if you
proceed the pool is gone.
So the button is useless or (better) should suggest you that it is going to remove all the
VMs and after confirmation proceed with the removal
Thanks,
michal
> --
> Shmuel
> _______________________________________________
> Devel mailing list
> Devel(a)ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
>
_______________________________________________
Devel mailing list
Devel(a)ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel
3527
days inactive
3528
days old
2 comments
3 participants
participants (3)
-
Michal Skrivanek -
Omer Frenkel -
Shmuel Melamud