DaanHoogland commented on code in PR #7545:
URL: https://github.com/apache/cloudstack/pull/7545#discussion_r1291130027
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/network/UpdateNetworkOfferingCmd.java:
##########
@@ -55,7 +55,7 @@ public class UpdateNetworkOfferingCmd extends BaseCmd {
private String displayText;
@Parameter(name = ApiConstants.AVAILABILITY, type = CommandType.STRING,
description = "the availability of network offering."
- + " Default value is Required for Guest Virtual network offering;
Optional for Guest Direct network offering")
+ + " The default value is Required for Guest Virtual network
offering, Optional for Guest Direct network offering. Only one network offering
can have the value Required ")
Review Comment:
It was not changed in this PR but I don ´t understand `Required for Guest
Virtual network offering, Optional for Guest Direct network offering`. it makes
the whole thing strange and incomprehensible.
Also I don't think it will make sense to users that the default is different
(as discribed) for the create vs the update API. I am not sure if we need to
change the code or the doc string, but more work is needed, i think.
##########
api/src/main/java/org/apache/cloudstack/api/command/admin/network/UpdateNetworkOfferingCmd.java:
##########
@@ -55,7 +55,7 @@ public class UpdateNetworkOfferingCmd extends BaseCmd {
private String displayText;
@Parameter(name = ApiConstants.AVAILABILITY, type = CommandType.STRING,
description = "the availability of network offering."
- + " Default value is Required for Guest Virtual network offering;
Optional for Guest Direct network offering")
+ + " The default value is Required for Guest Virtual network
offering, Optional for Guest Direct network offering. Only one network offering
can have the value Required ")
Review Comment:
It was not changed in this PR but I don ´t understand `Required for Guest
Virtual network offering, Optional for Guest Direct network offering`. it makes
the whole thing strange and incomprehensible.
Also I don't think it will make sense to users that the default is different
(as discribed) for the create vs the update API. I am not sure if we need to
change the code or the doc string, but more work is needed, i think.
--
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]
