DaanHoogland commented on code in PR #11350:
URL: https://github.com/apache/cloudstack/pull/11350#discussion_r2249201042
##########
api/src/main/java/org/apache/cloudstack/alert/AlertService.java:
##########
@@ -71,8 +71,8 @@ private AlertType(short type, String name, boolean isDefault)
{
public static final AlertType ALERT_TYPE_HA_ACTION = new
AlertType((short)30, "ALERT.HA.ACTION", true);
public static final AlertType ALERT_TYPE_CA_CERT = new
AlertType((short)31, "ALERT.CA.CERT", true);
public static final AlertType ALERT_TYPE_VM_SNAPSHOT = new
AlertType((short)32, "ALERT.VM.SNAPSHOT", true);
- public static final AlertType ALERT_TYPE_VR_PUBLIC_IFACE_MTU = new
AlertType((short)32, "ALERT.VR.PUBLIC.IFACE.MTU", true);
- public static final AlertType ALERT_TYPE_VR_PRIVATE_IFACE_MTU = new
AlertType((short)32, "ALERT.VR.PRIVATE.IFACE.MTU", true);
+ public static final AlertType ALERT_TYPE_VR_PUBLIC_IFACE_MTU = new
AlertType((short)33, "ALERT.VR.PUBLIC.IFACE.MTU", true);
+ public static final AlertType ALERT_TYPE_VR_PRIVATE_IFACE_MTU = new
AlertType((short)34, "ALERT.VR.PRIVATE.IFACE.MTU", true);
Review Comment:
like `Capacity.CAPACITY_TYPE_MEMORY` all these numbers should be mnenomic. I
think. an enum is in order, but a growable `Set` could be used as well, to make
it more dynamic. btw I don’t think the `Alert` types should be gotten from
external entities to this component (like `Capacity`) but external components
should at best be allowed to register types, or else use type provided by the
`AlertService` . </rant> I hope this makes sense.
Also all this said, this is not a rejection of this change, just some
remarks on the design quality/techdebt we are looking at.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
