GabrielBrascher commented on a change in pull request #4194:
URL: https://github.com/apache/cloudstack/pull/4194#discussion_r448413252



##########
File path: 
api/src/main/java/org/apache/cloudstack/api/command/admin/offering/UpdateDiskOfferingCmd.java
##########
@@ -77,6 +77,13 @@
             since = "4.13")
     private String zoneIds;
 
+    @Parameter(name = ApiConstants.TAGS,
+            type = CommandType.STRING,
+            description = "Comma-separated list of tags to be added to disk 
offering",

Review comment:
       > Comma-separated list of tags to be added to disk offering
   
   What do you think of changing this description to somethings as:
   _"comma-separated list of tags for the disk offering, tags should match with 
existing storage pool tags"_

##########
File path: 
server/src/main/java/com/cloud/configuration/ConfigurationManagerImpl.java
##########
@@ -3207,30 +3208,13 @@ public DiskOffering updateDiskOffering(final 
UpdateDiskOfferingCmd cmd) {
             diskOffering.setDisplayOffering(displayDiskOffering);
         }
 
-        // Note: tag editing commented out for now;keeping the code intact,
-        // might need to re-enable in next releases
-        // if (tags != null)
-        // {
-        // if (tags.trim().isEmpty() && diskOfferingHandle.getTags() == null)
-        // {
-        // //no new tags; no existing tags
-        // diskOffering.setTagsArray(csvTagsToList(null));
-        // }
-        // else if (!tags.trim().isEmpty() && diskOfferingHandle.getTags() !=
-        // null)
-        // {
-        // //new tags + existing tags
-        // List<String> oldTags = csvTagsToList(diskOfferingHandle.getTags());
-        // List<String> newTags = csvTagsToList(tags);
-        // oldTags.addAll(newTags);
-        // diskOffering.setTagsArray(oldTags);
-        // }
-        // else if(!tags.trim().isEmpty())
-        // {
-        // //new tags; NO existing tags
-        // diskOffering.setTagsArray(csvTagsToList(tags));
-        // }
-        // }
+        if (tags != null){
+            if(tags.isBlank()){
+                diskOffering.setTags(null);
+            }else {
+                diskOffering.setTags(tags);
+            }
+        }

Review comment:
       Can you please update the code formation according to the [CloudStack 
coding 
conventions](https://cwiki.apache.org/confluence/display/CLOUDSTACK/Coding+conventions)?
   
   Example of code formated:
   ```
   if (tags != null) {
       if (tags.isBlank()) {
           diskOffering.setTags(null);
       } else {
           diskOffering.setTags(tags);
       }
   }
   ```
   
   Documentation: 
https://cwiki.apache.org/confluence/display/CLOUDSTACK/Coding+conventions




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to